diff --git a/.audition-baseline.json b/.audition-baseline.json index fa1677629..1c19fba37 100644 --- a/.audition-baseline.json +++ b/.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, diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index ebf3163de..e3902c099 100644 --- a/docs/CHANGELOG.md +++ b/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* diff --git a/lib/view_component/base.rb b/lib/view_component/base.rb index 21b0d5460..81947954a 100644 --- a/lib/view_component/base.rb +++ b/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,7 +785,7 @@ 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 @@ -793,14 +793,35 @@ 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) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 3cfbdef5d..6d6718cef 100644 --- a/lib/view_component/compiler.rb +++ b/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! diff --git a/test/sandbox/test/collection_test.rb b/test/sandbox/test/collection_test.rb index 9b10d9afb..22b45e312 100644 --- a/test/sandbox/test/collection_test.rb +++ b/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 + "

#{@item.name} #{@counter}

".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 + "

\#{@product.name}

".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