From 2a37aa6b6a0e9c8765b9c0d219a5f66b894de575 Mon Sep 17 00:00:00 2001 From: Jakob Skjerning Date: Mon, 23 Jan 2023 19:16:43 +0100 Subject: [PATCH] Don't call super in component constructors We need to slurp all remaining options in the constructor, so that we can pass them on to the wrapper tag. However, ViewComponent::Base#initialize is defined as def initialize(*); end which messes up keyword argument slurping using the double splat operator. Given the following component constructor def initialize(foo:, **rest) super p rest end I'd expect the behavior to be ThatComponent.new(foo: 1) # {} given that the foo keyword argument is explicitly defined. However the actual behavior is ThatComponent.new(foo: 1) # {:foo => 1} If we don't call super we get the expected behavior: def initialize(foo:, **rest) p rest end ThatComponent.new(foo: 1) # {} However, not calling super is a bit of an anti-pattern (as discussed in https://github.com/rubocop/ruby-style-guide/issues/809), so it would be beneficial to be able to call it still. We cannot get by with slurping unknown keyword arguments before calling super, though: def initialize(foo:, **rest) p rest super p rest end somehow still messes with the local variable: ThatComponent.new(foo: 1) # {} # {:foo => 1} --- lib/input_group/email_field.rb | 4 +--- lib/input_group/text_field.rb | 4 +--- test/input_group/test_email_field.rb | 7 +++++++ test/input_group/test_text_field.rb | 10 ++++++++++ 4 files changed, 19 insertions(+), 6 deletions(-) diff --git a/lib/input_group/email_field.rb b/lib/input_group/email_field.rb index f36ced7..88937c0 100644 --- a/lib/input_group/email_field.rb +++ b/lib/input_group/email_field.rb @@ -76,14 +76,12 @@ def hint? # of the input group. See ActionView::Helpers::TagHelper#content_tag for # details. def initialize(attribute:, form:, help: nil, hint: nil, label: nil, placeholder: nil, **options) - super - @attribute = attribute @form = form @help = help @hint = hint @label = label - @options = options.symbolize_keys.except(:attribute, :form) + @options = options @placeholder = placeholder end diff --git a/lib/input_group/text_field.rb b/lib/input_group/text_field.rb index e7fa5b0..a2a334e 100644 --- a/lib/input_group/text_field.rb +++ b/lib/input_group/text_field.rb @@ -76,14 +76,12 @@ def hint? # of the input group. See ActionView::Helpers::TagHelper#content_tag for # details. def initialize(attribute:, form:, help: nil, hint: nil, label: nil, placeholder: nil, **options) - super - @attribute = attribute @form = form @help = help @hint = hint @label = label - @options = options.symbolize_keys.except(:attribute, :form) + @options = options @placeholder = placeholder end diff --git a/test/input_group/test_email_field.rb b/test/input_group/test_email_field.rb index 6b988d8..61b7ed5 100644 --- a/test/input_group/test_email_field.rb +++ b/test/input_group/test_email_field.rb @@ -148,6 +148,13 @@ def test_options_can_be_added_to_wrapping_element assert_css("div#subscriber-card") end + def test_known_options_are_not_added_to_wrapping_element + render_component_to_html + + refute_css("div[form]") + refute_css("div[attribute]") + end + def test_uses_configured_classes_for_wrapping_element Felt.configure do |config| config.classes = { diff --git a/test/input_group/test_text_field.rb b/test/input_group/test_text_field.rb index 1c92bbd..fc15ea8 100644 --- a/test/input_group/test_text_field.rb +++ b/test/input_group/test_text_field.rb @@ -168,6 +168,16 @@ def test_options_can_be_added_to_wrapping_element assert_css("div#game-card") end + def test_known_options_are_not_added_to_wrapping_element + model = Game.new + form = build_form(model) + + do_render(form) + + refute_css("div[form]") + refute_css("div[attribute]") + end + def test_uses_configured_classes_for_wrapping_element model = Game.new form = build_form(model)