Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: rendering optional attributes #31

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kortirso
Copy link
Contributor

@jho406 in continue #30

json.optional! - defines attribute or structure and marks it as optional, during serialization this method checks condition and renders or does not render such attribute

example of usage

json.optional! :properties do
  json.optional! :value, -> { object.value } # lambda here is to avoid value calculations before checking conditions for rendering
end

Works with partials, I just think how optionals can be used in simple template

@jho406 jho406 mentioned this pull request Jul 30, 2024
@jho406
Copy link
Contributor

jho406 commented Oct 14, 2024

Thanks for this PR! Out of the 3 PRs, i think this is one that i'm not fully ready for. Even if its manual, I do prefer the explicitness of if statements so I would actually do

if object.value
  json.properties do
    json.value object.value
  end
end

Other thoughts I have are.

  1. Does it work with digging?
  2. If i can mix json.optional! with the normal setters like json.header. For example, I wonder if this would work
json.optional! :properties do
  json.greeting "hello"
end

I think i'll leave this PR open for a bit to see if there are other commenters, but I think your approach in the meantime is a good path, extend props_template.

I also wonder if we can modify the syntax somehow using the block extension manager:

json.properties(optional: true) do
    json.value(object.value) if object.value
end

@jho406 jho406 self-requested a review as a code owner December 31, 2024 19:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants