Skip to content

Commit

Permalink
Fix slot names that start with call (#1828)
Browse files Browse the repository at this point in the history
* Fix slot names that start with call

When a component is registered that starts with `call` the compiler detects it as a template method which is not correct. This changes the compiler to look for `call(_|$)` instead of `call` to avoid this issue.

This means that slots can't (and couldn't) start with `call_` since we still rely on the `call_` naming convention to generate the template methods. To make this more dev friendly, this adds a check that raises an error if a slot name starts with `call_`.

A more long-term fix would be to use some kind of template method container instead of relying on the `call_` naming convention.
  • Loading branch information
BlakeWilliams authored Nov 3, 2023
1 parent 2bc5763 commit ad27702
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 5 deletions.
4 changes: 4 additions & 0 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ nav_order: 5

## main

* Resolve an issue where slots starting with `call` would cause a `NameError`

*Blake Williams*

* Add `use_helper` API.

*Reegan Viljoen*
Expand Down
4 changes: 2 additions & 2 deletions lib/view_component/compiler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -219,12 +219,12 @@ def inline_calls
component_class.included_modules
)

view_component_ancestors.flat_map { |ancestor| ancestor.instance_methods(false).grep(/^call/) }.uniq
view_component_ancestors.flat_map { |ancestor| ancestor.instance_methods(false).grep(/^call(_|$)/) }.uniq
end
end

def inline_calls_defined_on_self
@inline_calls_defined_on_self ||= component_class.instance_methods(false).grep(/^call/)
@inline_calls_defined_on_self ||= component_class.instance_methods(false).grep(/^call(_|$)/)
end

def variants
Expand Down
9 changes: 6 additions & 3 deletions lib/view_component/errors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,10 @@ class InvalidSlotDefinitionError < BaseError
"string, or callable (that is proc, lambda, etc)"
end

class SlotPredicateNameError < StandardError
class InvalidSlotNameError < StandardError
end

class SlotPredicateNameError < InvalidSlotNameError
MESSAGE =
"COMPONENT declares a slot named SLOT_NAME, which ends with a question mark.\n\n" \
"This isn't allowed because the ViewComponent framework already provides predicate " \
Expand All @@ -126,7 +129,7 @@ def initialize(klass_name, slot_name)
end
end

class ReservedSingularSlotNameError < StandardError
class ReservedSingularSlotNameError < InvalidSlotNameError
MESSAGE =
"COMPONENT declares a slot named SLOT_NAME, which is a reserved word in the ViewComponent framework.\n\n" \
"To fix this issue, choose a different name."
Expand All @@ -136,7 +139,7 @@ def initialize(klass_name, slot_name)
end
end

class ReservedPluralSlotNameError < StandardError
class ReservedPluralSlotNameError < InvalidSlotNameError
MESSAGE =
"COMPONENT declares a slot named SLOT_NAME, which is a reserved word in the ViewComponent framework.\n\n" \
"To fix this issue, choose a different name."
Expand Down
8 changes: 8 additions & 0 deletions lib/view_component/slotable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,7 @@ def validate_plural_slot_name(slot_name)
raise ReservedPluralSlotNameError.new(name, slot_name)
end

raise_if_slot_conflicts_with_call(slot_name)
raise_if_slot_ends_with_question_mark(slot_name)
raise_if_slot_registered(slot_name)
end
Expand All @@ -308,6 +309,7 @@ def validate_singular_slot_name(slot_name)
raise ReservedSingularSlotNameError.new(name, slot_name)
end

raise_if_slot_conflicts_with_call(slot_name)
raise_if_slot_ends_with_question_mark(slot_name)
raise_if_slot_registered(slot_name)
end
Expand All @@ -322,6 +324,12 @@ def raise_if_slot_registered(slot_name)
def raise_if_slot_ends_with_question_mark(slot_name)
raise SlotPredicateNameError.new(name, slot_name) if slot_name.to_s.ends_with?("?")
end

def raise_if_slot_conflicts_with_call(slot_name)
if slot_name.start_with?("call_")
raise InvalidSlotNameError, "Slot cannot start with 'call_'. Please rename #{slot_name}"
end
end
end

def get_slot(slot_name)
Expand Down
22 changes: 22 additions & 0 deletions test/sandbox/test/slotable_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -719,4 +719,26 @@ def test_slot_with_content_shorthand

assert component.title.content?
end

def test_slot_names_cannot_start_with_call_
assert_raises ViewComponent::InvalidSlotNameError do
Class.new(ViewComponent::Base) do
renders_one :call_out_title
end
end

assert_raises ViewComponent::InvalidSlotNameError do
Class.new(ViewComponent::Base) do
renders_many :call_out_titles
end
end
end

def test_slot_names_can_start_with_call
assert_nothing_raised do
Class.new(ViewComponent::Base) do
renders_one :callhome_et
end
end
end
end

0 comments on commit ad27702

Please sign in to comment.