Merge pull request #35411 from rails/pass-locals-to-template

Pass locals in to the template object on construction

7 files changed, 46 insertions, 55 deletions

diff --git a/actionview/lib/action_view/file_template.rb b/actionview/lib/action_view/file_template.rb
index cdc077a01a..dea02176eb 100644
--- a/actionview/lib/action_view/file_template.rb
+++ b/actionview/lib/action_view/file_template.rb
@@ -22,11 +22,11 @@ module ActionView
     # to ensure that references to the template object can be marshalled as well. This means forgoing
     # the marshalling of the compiler mutex and instantiating that again on unmarshalling.
     def marshal_dump # :nodoc:
-      [ @identifier, @handler, @compiled, @original_encoding, @locals, @virtual_path, @updated_at, @format, @variant ]
+      [ @identifier, @handler, @compiled, @locals, @virtual_path, @updated_at, @format, @variant ]
     end
 
     def marshal_load(array) # :nodoc:
-      @identifier, @handler, @compiled, @original_encoding, @locals, @virtual_path, @updated_at, @format, @variant = *array
+      @identifier, @handler, @compiled, @locals, @virtual_path, @updated_at, @format, @variant = *array
       @compile_mutex = Mutex.new
     end
   end
diff --git a/actionview/lib/action_view/template.rb b/actionview/lib/action_view/template.rb
index f67c90de76..7f29dedd7c 100644
--- a/actionview/lib/action_view/template.rb
+++ b/actionview/lib/action_view/template.rb
@@ -122,24 +122,26 @@ module ActionView
 
     extend Template::Handlers
 
-    attr_accessor :locals, :virtual_path
-
     attr_reader :source, :identifier, :handler, :original_encoding, :updated_at
-    attr_reader :variable, :format, :variant
+    attr_reader :variable, :format, :variant, :locals, :virtual_path
 
-    def initialize(source, identifier, handler, format: nil, variant: nil, **details)
+    def initialize(source, identifier, handler, format: nil, variant: nil, locals: nil, virtual_path: nil, updated_at: Time.now)
       unless format
         ActiveSupport::Deprecation.warn "ActionView::Template#initialize requires a format parameter"
         format = :html
       end
 
+      unless locals
+        ActiveSupport::Deprecation.warn "ActionView::Template#initialize requires a locals parameter"
+        locals = []
+      end
+
       @source            = source
       @identifier        = identifier
       @handler           = handler
       @compiled          = false
-      @original_encoding = nil
-      @locals            = details[:locals] || []
-      @virtual_path      = details[:virtual_path]
+      @locals            = locals
+      @virtual_path      = virtual_path
 
       @variable = if @virtual_path
         base = @virtual_path[-1] == "/" ? "" : File.basename(@virtual_path)
@@ -147,12 +149,15 @@ module ActionView
         $1.to_sym
       end
 
-      @updated_at        = details[:updated_at] || Time.now
+      @updated_at        = updated_at
       @format            = format
       @variant           = variant
       @compile_mutex     = Mutex.new
     end
 
+    deprecate :original_encoding
+    deprecate def virtual_path=(_); end
+    deprecate def locals=(_); end
     deprecate def formats=(_); end
     deprecate def formats; Array(format); end
     deprecate def variants=(_); end
@@ -261,11 +266,11 @@ module ActionView
     # to ensure that references to the template object can be marshalled as well. This means forgoing
     # the marshalling of the compiler mutex and instantiating that again on unmarshalling.
     def marshal_dump # :nodoc:
-      [ @source, @identifier, @handler, @compiled, @original_encoding, @locals, @virtual_path, @updated_at, @format, @variant ]
+      [ @source, @identifier, @handler, @compiled, @locals, @virtual_path, @updated_at, @format, @variant ]
     end
 
     def marshal_load(array) # :nodoc:
-      @source, @identifier, @handler, @compiled, @original_encoding, @locals, @virtual_path, @updated_at, @format, @variant = *array
+      @source, @identifier, @handler, @compiled, @locals, @virtual_path, @updated_at, @format, @variant = *array
       @compile_mutex = Mutex.new
     end
 
diff --git a/actionview/lib/action_view/template/resolver.rb b/actionview/lib/action_view/template/resolver.rb
index 6f0bf4187e..1c463efbb2 100644
--- a/actionview/lib/action_view/template/resolver.rb
+++ b/actionview/lib/action_view/template/resolver.rb
@@ -143,14 +143,18 @@ module ActionView
 
     # Normalizes the arguments and passes it on to find_templates.
     def find_all(name, prefix = nil, partial = false, details = {}, key = nil, locals = [])
+      locals = locals.map(&:to_s).sort!.freeze
+
       cached(key, [name, prefix, partial], details, locals) do
-        find_templates(name, prefix, partial, details)
+        find_templates(name, prefix, partial, details, false, locals)
       end
     end
 
     def find_all_anywhere(name, prefix, partial = false, details = {}, key = nil, locals = [])
+      locals = locals.map(&:to_s).sort!.freeze
+
       cached(key, [name, prefix, partial], details, locals) do
-        find_templates(name, prefix, partial, details, true)
+        find_templates(name, prefix, partial, details, true, locals)
       end
     end
 
@@ -165,13 +169,8 @@ module ActionView
     # This is what child classes implement. No defaults are needed
     # because Resolver guarantees that the arguments are present and
     # normalized.
-    def find_templates(name, prefix, partial, details, outside_app_allowed = false)
-      raise NotImplementedError, "Subclasses must implement a find_templates(name, prefix, partial, details, outside_app_allowed = false) method"
-    end
-
-    # Helpers that builds a path. Useful for building virtual paths.
-    def build_path(name, prefix, partial)
-      Path.build(name, prefix, partial)
+    def find_templates(name, prefix, partial, details, outside_app_allowed = false, locals = [])
+      raise NotImplementedError, "Subclasses must implement a find_templates(name, prefix, partial, details, outside_app_allowed = false, locals = []) method"
     end
 
     # Handles templates caching. If a key is given and caching is on
@@ -180,23 +179,13 @@ module ActionView
     # resolver is fresher before returning it.
     def cached(key, path_info, details, locals)
       name, prefix, partial = path_info
-      locals = locals.map(&:to_s).sort!
 
       if key
         @cache.cache(key, name, prefix, partial, locals) do
-          decorate(yield, path_info, details, locals)
+          yield
         end
       else
-        decorate(yield, path_info, details, locals)
-      end
-    end
-
-    # Ensures all the resolver information is set in the template.
-    def decorate(templates, path_info, details, locals)
-      cached = nil
-      templates.each do |t|
-        t.locals         = locals
-        t.virtual_path ||= (cached ||= build_path(*path_info))
+        yield
       end
     end
   end
@@ -213,12 +202,12 @@ module ActionView
 
     private
 
-      def find_templates(name, prefix, partial, details, outside_app_allowed = false)
+      def find_templates(name, prefix, partial, details, outside_app_allowed = false, locals)
         path = Path.build(name, prefix, partial)
-        query(path, details, details[:formats], outside_app_allowed)
+        query(path, details, details[:formats], outside_app_allowed, locals)
       end
 
-      def query(path, details, formats, outside_app_allowed)
+      def query(path, details, formats, outside_app_allowed, locals)
         template_paths = find_template_paths_from_details(path, details)
         template_paths = reject_files_external_to_app(template_paths) unless outside_app_allowed
 
@@ -229,6 +218,7 @@ module ActionView
             virtual_path: path.virtual,
             format: format,
             variant: variant,
+            locals: locals,
             updated_at: mtime(template)
           )
         end
@@ -429,9 +419,5 @@ module ActionView
     def self.instances
       [new(""), new("/")]
     end
-
-    def decorate(*)
-      super.each { |t| t.virtual_path = nil }
-    end
   end
 end
diff --git a/actionview/lib/action_view/testing/resolvers.rb b/actionview/lib/action_view/testing/resolvers.rb
index 275e2e9182..2305fc9b81 100644
--- a/actionview/lib/action_view/testing/resolvers.rb
+++ b/actionview/lib/action_view/testing/resolvers.rb
@@ -23,7 +23,7 @@ module ActionView #:nodoc:
 
     private
 
-      def query(path, exts, _, _)
+      def query(path, exts, _, _, locals)
         query = +""
         EXTENSIONS.each_key do |ext|
           query << "(" << exts[ext].map { |e| e && Regexp.escape(".#{e}") }.join("|") << "|)"
@@ -39,6 +39,7 @@ module ActionView #:nodoc:
             virtual_path: path.virtual,
             format: format,
             variant: variant,
+            locals: locals,
             updated_at: updated_at
           )
         end
@@ -48,9 +49,9 @@ module ActionView #:nodoc:
   end
 
   class NullResolver < PathResolver
-    def query(path, exts, _, _)
+    def query(path, exts, _, _, locals)
       handler, format, variant = extract_handler_and_format_and_variant(path, :html)
-      [ActionView::Template.new("Template generated by Null Resolver", path.virtual, handler, virtual_path: path.virtual, format: format, variant: variant)]
+      [ActionView::Template.new("Template generated by Null Resolver", path.virtual, handler, virtual_path: path.virtual, format: format, variant: variant, locals: locals)]
     end
   end
 end
diff --git a/actionview/test/abstract_unit.rb b/actionview/test/abstract_unit.rb
index e0a1f59755..fe317bf39e 100644
--- a/actionview/test/abstract_unit.rb
+++ b/actionview/test/abstract_unit.rb
@@ -60,7 +60,7 @@ module RenderERBUtils
       string.strip,
       "test template",
       ActionView::Template.handler_for_extension(:erb),
-      format: :html)
+      format: :html, locals: [])
 
     view = ActionView::Base.with_empty_template_cache
     template.render(view.empty, {}).strip
diff --git a/actionview/test/actionpack/controller/view_paths_test.rb b/actionview/test/actionpack/controller/view_paths_test.rb
index a7a58a78cc..4449d08496 100644
--- a/actionview/test/actionpack/controller/view_paths_test.rb
+++ b/actionview/test/actionpack/controller/view_paths_test.rb
@@ -143,8 +143,9 @@ class ViewLoadPathsTest < ActionController::TestCase
             "Decorated body",
             template.identifier,
             template.handler,
-              virtual_path: template.virtual_path,
-              format: template.format
+            virtual_path: template.virtual_path,
+            format: template.format,
+            locals: template.locals
           )
         end
       end
diff --git a/actionview/test/template/template_test.rb b/actionview/test/template/template_test.rb
index c74305fb42..71fb99115b 100644
--- a/actionview/test/template/template_test.rb
+++ b/actionview/test/template/template_test.rb
@@ -39,7 +39,8 @@ class TestERBTemplate < ActiveSupport::TestCase
         "partial",
         ERBHandler,
         virtual_path: "partial",
-        format: :html
+        format: :html,
+        locals: []
       )
     end
 
@@ -57,8 +58,8 @@ class TestERBTemplate < ActiveSupport::TestCase
   end
 
   def new_template(body = "<%= hello %>", details = {})
-    details = { format: :html }.merge details
-    ActionView::Template.new(body.dup, "hello template", details.fetch(:handler) { ERBHandler }, { virtual_path: "hello" }.merge!(details))
+    details = { format: :html, locals: [] }.merge details
+    ActionView::Template.new(body.dup, "hello template", details.delete(:handler) || ERBHandler, { virtual_path: "hello" }.merge!(details))
   end
 
   def render(locals = {})
@@ -103,8 +104,7 @@ class TestERBTemplate < ActiveSupport::TestCase
   end
 
   def test_locals
-    @template = new_template("<%= my_local %>")
-    @template.locals = [:my_local]
+    @template = new_template("<%= my_local %>", locals: [:my_local])
     assert_equal "I am a local", render(my_local: "I am a local")
   end
 
@@ -122,16 +122,14 @@ class TestERBTemplate < ActiveSupport::TestCase
   end
 
   def test_refresh_with_templates
-    @template = new_template("Hello", virtual_path: "test/foo/bar")
-    @template.locals = [:key]
+    @template = new_template("Hello", virtual_path: "test/foo/bar", locals: [:key])
     assert_called_with(@context.lookup_context, :find_template, ["bar", %w(test/foo), false, [:key]], returns: "template") do
       assert_equal "template", @template.refresh(@context)
     end
   end
 
   def test_refresh_with_partials
-    @template = new_template("Hello", virtual_path: "test/_foo")
-    @template.locals = [:key]
+    @template = new_template("Hello", virtual_path: "test/_foo", locals: [:key])
     assert_called_with(@context.lookup_context, :find_template, ["foo", %w(test), true, [:key]], returns: "partial") do
       assert_equal "partial", @template.refresh(@context)
     end