Skip to content

Commit b35a9b6

Browse files
committed
ActionView: Layouts can access local variables
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
1 parent bc601a9 commit b35a9b6

File tree

4 files changed

+28
-12
lines changed

4 files changed

+28
-12
lines changed

actionview/CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,9 @@
1+
* Layouts have access to local variables passed to `render`.
2+
3+
This fixes #31680 which was a regression in Rails 5.1.
4+
5+
*Mike Dalessio*
6+
17
* Argument errors related to strict locals in templates now raise an
28
`ActionView::StrictLocalsError`, and all other argument errors are reraised as-is.
39

actionview/lib/action_view/layouts.rb

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,7 @@ def _write_layout_method # :nodoc:
284284
silence_redefinition_of_method(:_layout)
285285

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

326326
class_eval <<-RUBY, __FILE__, __LINE__ + 1
327327
# frozen_string_literal: true
328-
def _layout(lookup_context, formats)
328+
def _layout(lookup_context, formats, keys)
329329
if _conditional_layout?
330330
#{layout_definition}
331331
else
@@ -389,8 +389,8 @@ def _layout_for_option(name)
389389
case name
390390
when String then _normalize_layout(name)
391391
when Proc then name
392-
when true then Proc.new { |lookup_context, formats| _default_layout(lookup_context, formats, true) }
393-
when :default then Proc.new { |lookup_context, formats| _default_layout(lookup_context, formats, false) }
392+
when true then Proc.new { |lookup_context, formats, keys| _default_layout(lookup_context, formats, keys, true) }
393+
when :default then Proc.new { |lookup_context, formats, keys| _default_layout(lookup_context, formats, keys, false) }
394394
when false, nil then nil
395395
else
396396
raise ArgumentError,
@@ -412,9 +412,9 @@ def _normalize_layout(value)
412412
#
413413
# ==== Returns
414414
# * <tt>template</tt> - The template object for the default layout (or +nil+)
415-
def _default_layout(lookup_context, formats, require_layout = false)
415+
def _default_layout(lookup_context, formats, keys, require_layout = false)
416416
begin
417-
value = _layout(lookup_context, formats) if action_has_layout?
417+
value = _layout(lookup_context, formats, keys) if action_has_layout?
418418
rescue NameError => e
419419
raise e, "Could not render layout: #{e.message}"
420420
end

actionview/lib/action_view/renderer/template_renderer.rb

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -99,14 +99,14 @@ def resolve_layout(layout, keys, formats)
9999
if layout.start_with?("/")
100100
raise ArgumentError, "Rendering layouts from an absolute path is not supported."
101101
else
102-
@lookup_context.find_template(layout, nil, false, [], details)
102+
@lookup_context.find_template(layout, nil, false, keys, details)
103103
end
104104
rescue ActionView::MissingTemplate
105105
all_details = @details.merge(formats: @lookup_context.default_formats)
106-
raise unless template_exists?(layout, nil, false, [], **all_details)
106+
raise unless template_exists?(layout, nil, false, keys, **all_details)
107107
end
108108
when Proc
109-
resolve_layout(layout.call(@lookup_context, formats), keys, formats)
109+
resolve_layout(layout.call(@lookup_context, formats, keys), keys, formats)
110110
else
111111
layout
112112
end

actionview/test/actionpack/abstract/layouts_test.rb

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ class Base < AbstractController::Base
1515
self.view_paths = [ActionView::FixtureResolver.new(
1616
"some/template.erb" => "hello <%= foo %> bar",
1717
"layouts/hello.erb" => "With String <%= yield %>",
18-
"layouts/hello_locals.erb" => "With String <%= yield %>",
18+
"layouts/hello_locals.erb" => "With String <%= foo %> <%= yield %>",
1919
"layouts/hello_override.erb" => "With Override <%= yield %>",
2020
"layouts/overwrite.erb" => "Overwrite <%= yield %>",
2121
"layouts/with_false_layout.erb" => "False Layout <%= yield %>",
@@ -41,6 +41,10 @@ class WithStringLocals < Base
4141
def index
4242
render template: "some/template", locals: { foo: "less than 3" }
4343
end
44+
45+
def overwrite_with_default
46+
render layout: :default, template: "some/template", locals: { foo: "less than 3" }
47+
end
4448
end
4549

4650
class WithString < Base
@@ -279,10 +283,16 @@ class TestBase < ActiveSupport::TestCase
279283
assert_equal "Hello blank!", controller.response_body
280284
end
281285

282-
test "with locals" do
286+
test "when layout is specified as a string, render with locals" do
283287
controller = WithStringLocals.new
284288
controller.process(:index)
285-
assert_equal "With String hello less than 3 bar", controller.response_body
289+
assert_equal "With String less than 3 hello less than 3 bar", controller.response_body
290+
end
291+
292+
test "when layout is specified as a string, overwriting with :default, render with locals" do
293+
controller = WithStringLocals.new
294+
controller.process(:overwrite_with_default)
295+
assert_equal "With String less than 3 hello less than 3 bar", controller.response_body
286296
end
287297

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

0 commit comments

Comments
 (0)