Merge pull request #2724 from bensheldon/collection-arguments-ractor-safety

Fix stale and inherited collection parameter detection; build it at compile time

Author: joelhawksley

.audition-baseline.json

@@ -1,5 +1,5 @@
 {
-  "class-level-state|lib/view_component/base.rb": 12,
+  "class-level-state|lib/view_component/base.rb": 7,
   "class-level-state|lib/view_component/cache_digest.rb": 1,
   "class-level-state|lib/view_component/configurable.rb": 1,
   "class-level-state|lib/view_component/engine.rb": 1,

docs/CHANGELOG.md

@@ -10,6 +10,10 @@ nav_order: 6
 
 ## main
 
+* Fix subclasses defined after their parent renders a collection using the parent's default collection parameter name instead of their own. Fix recompiling a component not refreshing its collection parameter, counter, and iteration detection. Compute that detection when a component compiles, and store the results frozen so they're shareable across Ractors.
+
+    *Ben Sheldon*
+
 * Fix collection counter and iteration detection, and collection and reserved parameter validation, when `initialize` arguments are forwarded through anonymous arguments (`...` or `**`), such as by prepended modules and subclasses calling `super`.
 
     *Ben Sheldon*

lib/view_component/base.rb

@@ -747,7 +747,7 @@ def strip_trailing_whitespace?
       # @private
       def __vc_validate_collection_parameter!(validate_default: false)
         if validate_default
-          return if __vc_collection_arguments[:parameter]
+          return if __vc_collection_arguments[:parameter_accepted]
 
           raise MissingCollectionArgumentError.new(name, __vc_collection_parameter)
         end
@@ -770,12 +770,12 @@ def __vc_validate_initialization_parameters!
 
       # @private
       def __vc_collection_parameter
-        @__vc_provided_collection_parameter ||= name && name.demodulize.underscore.chomp("_component").to_sym
+        __vc_collection_arguments[:parameter]
       end
 
       # @private
       def __vc_collection_counter_parameter
-        @__vc_collection_counter_parameter ||= :"#{__vc_collection_parameter}_counter"
+        __vc_collection_arguments[:counter_parameter]
       end
 
       # @private
@@ -785,22 +785,43 @@ def __vc_counter_argument_present?
 
       # @private
       def __vc_collection_iteration_parameter
-        @__vc_collection_iteration_parameter ||= :"#{__vc_collection_parameter}_iteration"
+        __vc_collection_arguments[:iteration_parameter]
       end
 
       # @private
       def __vc_iteration_argument_present?
         __vc_collection_arguments[:iteration]
       end
 
+      # @private
+      def __vc_build_collection_arguments
+        names =
+          if respond_to?(:attribute_names)
+            attribute_names.map(&:to_sym)
+          else
+            __vc_initialize_signatures.flat_map { |_owner, parameters| parameters.map(&:last) }
+          end
+        parameter = __vc_provided_collection_parameter || (name && name.demodulize.underscore.chomp("_component").to_sym)
+        counter_parameter = :"#{parameter}_counter"
+        iteration_parameter = :"#{parameter}_iteration"
+
+        @__vc_collection_arguments = Ractor.make_shareable(
+          {
+            initialize_parameter_names: names,
+            parameter: parameter,
+            parameter_accepted: parameter.nil? || names.include?(parameter) || __vc_splatted_keyword_argument_present?,
+            counter_parameter: counter_parameter,
+            counter: names.include?(counter_parameter),
+            iteration_parameter: iteration_parameter,
+            iteration: names.include?(iteration_parameter)
+          }
+        )
+      end
+
       private
 
       def __vc_collection_arguments
-        @__vc_collection_arguments ||= {
-          parameter: __vc_collection_parameter_accepted?(__vc_collection_parameter),
-          counter: __vc_initialize_parameter_names.include?(__vc_collection_counter_parameter),
-          iteration: __vc_initialize_parameter_names.include?(__vc_collection_iteration_parameter)
-        }.freeze
+        @__vc_collection_arguments || __vc_build_collection_arguments
       end
 
       def __vc_collection_parameter_accepted?(parameter)
@@ -818,12 +839,7 @@ def __vc_splatted_keyword_argument_present?
       end
 
       def __vc_initialize_parameter_names
-        @__vc_initialize_parameter_names ||=
-          if respond_to?(:attribute_names)
-            attribute_names.map(&:to_sym)
-          else
-            __vc_initialize_signatures.flat_map { |_owner, parameters| parameters.map(&:last) }
-          end
+        __vc_collection_arguments[:initialize_parameter_names]
       end
 
       def __vc_initialize_signatures
@@ -838,9 +854,7 @@ def __vc_initialize_signatures
         signatures
       end
 
-      def __vc_provided_collection_parameter
-        @__vc_provided_collection_parameter ||= nil
-      end
+      attr_reader :__vc_provided_collection_parameter
     end
 
     ActiveSupport.run_load_hooks(:view_component, self)

lib/view_component/compiler.rb

@@ -41,6 +41,8 @@ def compile(raise_errors: false, force: false)
           return false
         end
 
+        @component.__vc_build_collection_arguments
+
         if raise_errors
           @component.__vc_validate_initialization_parameters!
           @component.__vc_validate_collection_parameter!

test/sandbox/test/collection_test.rb

@@ -224,5 +224,97 @@ def test_prepended_named_keyword_splat_accepts_collection_parameter
       assert_selector("p", text: "Radio clock")
       assert_selector("p", text: "Mints")
     end
+
+    def test_collection_arguments_are_ractor_shareable
+      ForwardedCounterComponent.__vc_compile
+
+      assert Ractor.shareable?(ForwardedCounterComponent.send(:__vc_collection_arguments))
+    end
+
+    class RecompiledCounterComponent < ViewComponent::Base
+      with_collection_parameter :item
+
+      def initialize(item:)
+        @item = item
+      end
+
+      def call
+        "<p>#{@item.name} #{@counter}</p>".html_safe
+      end
+    end
+
+    def test_recompiling_refreshes_collection_arguments
+      render_inline(RecompiledCounterComponent.with_collection(@products))
+      RecompiledCounterComponent.prepend(ConsumeCounter)
+      RecompiledCounterComponent.__vc_compile(force: true)
+
+      render_inline(RecompiledCounterComponent.with_collection(@products))
+
+      assert_selector("p", text: "Radio clock 0")
+      assert_selector("p", text: "Mints 1")
+    end
+
+    class DefaultParameterParentComponent < ViewComponent::Base
+      def initialize(**)
+      end
+
+      def call
+        ""
+      end
+    end
+
+    def test_subclass_defined_after_parent_renders_uses_its_own_collection_parameter
+      render_inline(DefaultParameterParentComponent.with_collection(@products))
+
+      self.class.class_eval(<<~RUBY, __FILE__, __LINE__ + 1)
+        class DefaultParameterChildComponent < DefaultParameterParentComponent
+          def initialize(default_parameter_child:)
+            @product = default_parameter_child
+          end
+
+          def call
+            "<p>\#{@product.name}</p>".html_safe
+          end
+        end
+      RUBY
+
+      assert_equal :default_parameter_child, DefaultParameterChildComponent.__vc_collection_parameter
+
+      render_inline(DefaultParameterChildComponent.with_collection(@products))
+
+      assert_selector("p", text: "Radio clock")
+      assert_selector("p", text: "Mints")
+    end
+
+    class ReservedParameterParentComponent < ViewComponent::Base
+      def initialize(title:, content: nil)
+      end
+
+      def call
+        ""
+      end
+    end
+
+    class NamedKeywordSplatChildComponent < ReservedParameterParentComponent
+      def initialize(**options)
+        super(title: options[:title])
+      end
+    end
+
+    class AnonymousKeywordSplatChildComponent < ReservedParameterParentComponent
+      def initialize(**)
+        super
+      end
+    end
+
+    def test_keyword_splat_subclass_raises_for_reserved_parameter_in_parent
+      assert_raises(ViewComponent::ReservedParameterError) do
+        NamedKeywordSplatChildComponent.__vc_validate_initialization_parameters!
+      end
+
+      assert_raises(ViewComponent::ReservedParameterError) do
+        AnonymousKeywordSplatChildComponent.__vc_validate_initialization_parameters!
+      end
+    end
   end
 end