Skip to content

nil-collection should be treated as empty array in template #148

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

Merged
merged 1 commit into from
Oct 9, 2013
Merged

nil-collection should be treated as empty array in template #148

merged 1 commit into from
Oct 9, 2013

Conversation

garysweaver
Copy link
Contributor

I noticed that when I passed in a nil as an empty collection in a template, it did not treat as an empty collection, although a nil-collection was already treated as an empty array in one place already in JBuilder:

json.array! @posts, partial: 'posts/post', as: :post
json.partial! 'posts/post', collection: @posts, as: :post
json.partial! partial: 'posts/post', collection: @posts, as: :post
json.comments @post.comments, partial: 'comment/comment', as: :comment

Without this change, if you were to have json.array! @foos, partial: 'path/to/partial/foo', as: :foo in index view and json.(foo, ...) in partial view, it would use the partial template that will return an error like the following, which is non-obvious; I had no idea that when the collection was nil it would still try to render the partial, so I thought it had to be something else:

NameError: undefined local variable or method `foo' for <<Class:0x007fc699c747c0>:0x007fc69a6e42f0>
app/views/path/to/partial/_foo.json.jbuilder:1:in `_app_views_path_to_partial__foo_json_jbuilder___82125411139652069_70245417019000'
app/views/path/to/view/index.json.jbuilder:1:in `_app_views_path_to_view_index_json_jbuilder__40707230780250694_70245416948080'

Instead of treating a nil collection as an empty array, it could raise ArgumentError or similar with a more specific error, but the following seems to go against that a little:

  • _map_collection treating nil collection as array here and test here.
  • set! (method_missing handling to allow json.* calls in template) seeming to make an assumption here that something that doesn't respond to map is a single value and not array. So, I changed to look for :as option also.

Thanks for considering this change. We can work around it by having nil checks in each template, but I don't think that is as obvious.

rwz added a commit that referenced this pull request Oct 9, 2013
…tempt_to_use_partial

nil-collection should be treated as empty array in template
@rwz rwz merged commit dda5f96 into rails:master Oct 9, 2013
@rwz
Copy link
Collaborator

rwz commented Oct 9, 2013

Thanks for fixing it!

@rwz
Copy link
Collaborator

rwz commented Oct 9, 2013

Released 1.5.2

@garysweaver garysweaver deleted the nil_collection_should_not_attempt_to_use_partial branch October 23, 2013 15:47
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