Skip to content

Commit

Permalink
Don't call super in component constructors
Browse files Browse the repository at this point in the history
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
rubocop/ruby-style-guide#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}
  • Loading branch information
koppen committed Jan 23, 2023
1 parent f0b3864 commit 2a37aa6
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 6 deletions.
4 changes: 1 addition & 3 deletions lib/input_group/email_field.rb
Expand Up @@ -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

Expand Down
4 changes: 1 addition & 3 deletions lib/input_group/text_field.rb
Expand Up @@ -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

Expand Down
7 changes: 7 additions & 0 deletions test/input_group/test_email_field.rb
Expand Up @@ -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 = {
Expand Down
10 changes: 10 additions & 0 deletions test/input_group/test_text_field.rb
Expand Up @@ -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)
Expand Down

0 comments on commit 2a37aa6

Please sign in to comment.