Skip to content

Commit

Permalink
ActionView: Layouts can access local variables
Browse files Browse the repository at this point in the history
Local variable access was originally broken, reportedly, in Rails 5.1
in d6bac04 which dropped the `locals` keys from some of the layout
lookup methods to optimize the template resolver cache.

In the meantime, the template resolver cache was discarded in Rails
7.0 in 4db7ae5, and was replaced by a FileSystemResolver cache that
does not use the locals in the lookup key (because it's storing
unbound templates).

So this fix is, essentially, to once again pass the local variable
names through the many layers of the layout resolver/renderer stack to
make sure that when the unbound template is found and `#bind_locals`
is called on it, it's bound with the proper set of local variable
names.

Fixes rails#31680
  • Loading branch information
flavorjones committed Jan 6, 2025
1 parent bc601a9 commit b35a9b6
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 12 deletions.
6 changes: 6 additions & 0 deletions actionview/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
* Layouts have access to local variables passed to `render`.

This fixes #31680 which was a regression in Rails 5.1.

*Mike Dalessio*

* Argument errors related to strict locals in templates now raise an
`ActionView::StrictLocalsError`, and all other argument errors are reraised as-is.

Expand Down
12 changes: 6 additions & 6 deletions actionview/lib/action_view/layouts.rb
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ def _write_layout_method # :nodoc:
silence_redefinition_of_method(:_layout)

prefixes = /\blayouts/.match?(_implied_layout_name) ? [] : ["layouts"]
default_behavior = "lookup_context.find_all('#{_implied_layout_name}', #{prefixes.inspect}, false, [], { formats: formats }).first || super"
default_behavior = "lookup_context.find_all('#{_implied_layout_name}', #{prefixes.inspect}, false, keys, { formats: formats }).first || super"
name_clause = if name
default_behavior
else
Expand Down Expand Up @@ -325,7 +325,7 @@ def _write_layout_method # :nodoc:

class_eval <<-RUBY, __FILE__, __LINE__ + 1
# frozen_string_literal: true
def _layout(lookup_context, formats)
def _layout(lookup_context, formats, keys)
if _conditional_layout?
#{layout_definition}
else
Expand Down Expand Up @@ -389,8 +389,8 @@ def _layout_for_option(name)
case name
when String then _normalize_layout(name)
when Proc then name
when true then Proc.new { |lookup_context, formats| _default_layout(lookup_context, formats, true) }
when :default then Proc.new { |lookup_context, formats| _default_layout(lookup_context, formats, false) }
when true then Proc.new { |lookup_context, formats, keys| _default_layout(lookup_context, formats, keys, true) }
when :default then Proc.new { |lookup_context, formats, keys| _default_layout(lookup_context, formats, keys, false) }
when false, nil then nil
else
raise ArgumentError,
Expand All @@ -412,9 +412,9 @@ def _normalize_layout(value)
#
# ==== Returns
# * <tt>template</tt> - The template object for the default layout (or +nil+)
def _default_layout(lookup_context, formats, require_layout = false)
def _default_layout(lookup_context, formats, keys, require_layout = false)
begin
value = _layout(lookup_context, formats) if action_has_layout?
value = _layout(lookup_context, formats, keys) if action_has_layout?
rescue NameError => e
raise e, "Could not render layout: #{e.message}"
end
Expand Down
6 changes: 3 additions & 3 deletions actionview/lib/action_view/renderer/template_renderer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -99,14 +99,14 @@ def resolve_layout(layout, keys, formats)
if layout.start_with?("/")
raise ArgumentError, "Rendering layouts from an absolute path is not supported."
else
@lookup_context.find_template(layout, nil, false, [], details)
@lookup_context.find_template(layout, nil, false, keys, details)
end
rescue ActionView::MissingTemplate
all_details = @details.merge(formats: @lookup_context.default_formats)
raise unless template_exists?(layout, nil, false, [], **all_details)
raise unless template_exists?(layout, nil, false, keys, **all_details)
end
when Proc
resolve_layout(layout.call(@lookup_context, formats), keys, formats)
resolve_layout(layout.call(@lookup_context, formats, keys), keys, formats)
else
layout
end
Expand Down
16 changes: 13 additions & 3 deletions actionview/test/actionpack/abstract/layouts_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class Base < AbstractController::Base
self.view_paths = [ActionView::FixtureResolver.new(
"some/template.erb" => "hello <%= foo %> bar",
"layouts/hello.erb" => "With String <%= yield %>",
"layouts/hello_locals.erb" => "With String <%= yield %>",
"layouts/hello_locals.erb" => "With String <%= foo %> <%= yield %>",
"layouts/hello_override.erb" => "With Override <%= yield %>",
"layouts/overwrite.erb" => "Overwrite <%= yield %>",
"layouts/with_false_layout.erb" => "False Layout <%= yield %>",
Expand All @@ -41,6 +41,10 @@ class WithStringLocals < Base
def index
render template: "some/template", locals: { foo: "less than 3" }
end

def overwrite_with_default
render layout: :default, template: "some/template", locals: { foo: "less than 3" }
end
end

class WithString < Base
Expand Down Expand Up @@ -279,10 +283,16 @@ class TestBase < ActiveSupport::TestCase
assert_equal "Hello blank!", controller.response_body
end

test "with locals" do
test "when layout is specified as a string, render with locals" do
controller = WithStringLocals.new
controller.process(:index)
assert_equal "With String hello less than 3 bar", controller.response_body
assert_equal "With String less than 3 hello less than 3 bar", controller.response_body
end

test "when layout is specified as a string, overwriting with :default, render with locals" do
controller = WithStringLocals.new
controller.process(:overwrite_with_default)
assert_equal "With String less than 3 hello less than 3 bar", controller.response_body
end

test "when layout is specified as a string, render with that layout" do
Expand Down

0 comments on commit b35a9b6

Please sign in to comment.