Skip to content

Commit

Permalink
Suggestion: Return early from collection partial rendering if blank. (#…
Browse files Browse the repository at this point in the history
…560)

* Suggestion: Return early from collection partial rendering if blank.

Co-authored-by: David Heinemeier Hansson <[email protected]>
  • Loading branch information
tylerjc and dhh authored Sep 14, 2024
1 parent 8dbc23f commit 0adeb96
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 11 deletions.
31 changes: 20 additions & 11 deletions lib/jbuilder/jbuilder_template.rb
Original file line number Diff line number Diff line change
Expand Up @@ -155,21 +155,30 @@ def _render_partial_with_options(options)
::Kernel.raise ::NotImplementedError, "The `:spacer_template' option is not supported in collection rendering."
end

results = CollectionRenderer
.new(@context.lookup_context, options) { |&block| _scope(&block) }
.render_collection_with_partial(collection, partial, @context, nil)

array! if results.respond_to?(:body) && results.body.nil?
if collection.present?
results = CollectionRenderer
.new(@context.lookup_context, options) { |&block| _scope(&block) }
.render_collection_with_partial(collection, partial, @context, nil)

array! if results.respond_to?(:body) && results.body.nil?
else
array!
end
elsif as && options.key?(:collection) && !CollectionRenderer.supported?
# For Rails <= 5.2:
as = as.to_sym
collection = options.delete(:collection)
locals = options.delete(:locals)
array! collection do |member|
member_locals = locals.clone
member_locals.merge! collection: collection
member_locals.merge! as => member
_render_partial options.merge(locals: member_locals)

if collection.present?
locals = options.delete(:locals)
array! collection do |member|
member_locals = locals.clone
member_locals.merge! collection: collection
member_locals.merge! as => member
_render_partial options.merge(locals: member_locals)
end
else
array!
end
else
_render_partial options
Expand Down
16 changes: 16 additions & 0 deletions test/jbuilder_template_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,12 @@ class JbuilderTemplateTest < ActiveSupport::TestCase
end

test "nil partial collection by name" do
Jbuilder::CollectionRenderer.expects(:new).never
assert_equal [], render('json.partial! "post", collection: @posts, as: :post', posts: nil)
end

test "nil partial collection by options" do
Jbuilder::CollectionRenderer.expects(:new).never
assert_equal [], render('json.partial! partial: "post", collection: @posts, as: :post', posts: nil)
end

Expand All @@ -113,7 +115,13 @@ class JbuilderTemplateTest < ActiveSupport::TestCase
assert_equal "Pavel", result[5]["author"]["first_name"]
end

test "empty array of partials from empty collection" do
Jbuilder::CollectionRenderer.expects(:new).never
assert_equal [], render('json.array! @posts, partial: "post", as: :post', posts: [])
end

test "empty array of partials from nil collection" do
Jbuilder::CollectionRenderer.expects(:new).never
assert_equal [], render('json.array! @posts, partial: "post", as: :post', posts: nil)
end

Expand All @@ -126,10 +134,17 @@ class JbuilderTemplateTest < ActiveSupport::TestCase
end

test "empty array of partials under key from nil collection" do
Jbuilder::CollectionRenderer.expects(:new).never
result = render('json.posts @posts, partial: "post", as: :post', posts: nil)
assert_equal [], result["posts"]
end

test "empty array of partials under key from an empy collection" do
Jbuilder::CollectionRenderer.expects(:new).never
result = render('json.posts @posts, partial: "post", as: :post', posts: [])
assert_equal [], result["posts"]
end

test "object fragment caching" do
render(<<-JBUILDER)
json.cache! "cache-key" do
Expand Down Expand Up @@ -293,6 +308,7 @@ class JbuilderTemplateTest < ActiveSupport::TestCase

if JbuilderTemplate::CollectionRenderer.supported?
test "returns an empty array for an empty collection" do
Jbuilder::CollectionRenderer.expects(:new).never
result = render('json.array! @posts, partial: "post", as: :post, cached: true', posts: [])

# Do not use #assert_empty as it is important to ensure that the type of the JSON result is an array.
Expand Down

0 comments on commit 0adeb96

Please sign in to comment.