Separate slot setter and getter method #1184
Conversation
This introduces an explicit setter for slots in the form of
`with_#{slot_name}`. This makes it more clear when slots are being set
vs accessed but also allows chaining when slots have no arguments.
Before this change, the following would not work:
```ruby
component.header.with_content("my header!")
```
This fails due to `header` being passed no arguments and no block,
resulting in it acting as a getter, returning `nil` since the slot was
not defined. This is due to assuming that slots will always be passed
either a block or arguments and predates the `with_content` method.
Now, the above can be implemented using `with_*`:
```ruby
component.with_header.with_content("my header!")
```
|
I didn't add a deprecation notice in this PR, but I think we'll likely decide to deprecate using |
Co-authored-by: Hans Lemuet <Spone@users.noreply.github.com>
|
Hi @BlakeWilliams! I'm wondering if you considered keeping the setter unchanged, but namespacing the getter. My starting point here is trying to ease the migration of large codebases to the new syntax. I think the getter is nearly always used internally to the ViewComponent, while the setter is usually called in a lot of places around the app, wherever you render an instance of the ViewComponent with the given slot. So if only the getter is changed, you would only have to update each ViewComponent using slots (and its template(s)).
In terms of syntax, what about something like this: class MyComponent < ViewComponent::Base
renders_one :header
end
c = MyComponent.new
# setter
c.header { "Hello world!" }
c.header.with_content("Hello world!")
# getter (suggestions)
c.slots.header # or
c.header_slot # or
c.header_content |
|
@Spone that's a good point. I thought about it from that angle a bit, but I was struggling to come up with an API I was happy with. Your point about it being easier to migrate is a good one, but I'm not sure we should optimize for that over API ergonomics. Either way, I'm happy to chat about how we could inverse the API like you suggest. I'll comment in the ADR and share some thoughts. |
|
@BlakeWilliams I'd agree that the API proposed here has the right ergonomics and we should aim for that vs compromising it for the sake of upgrade ease. For me the trick is going to be to provide the right upgrade path. Perhaps a mechanism to opt in to the old behavior allowing consumers to upgrade legacy components at their leisure. |
| @@ -45,10 +45,16 @@ def register_polymorphic_slot(slot_name, types, collection:) | |||
| "#{slot_name}_#{poly_type}" | |||
| end | |||
|
|
|||
| # Deprecated: Will be removed in 3.0 | |||
There was a problem hiding this comment.
Can we add a deprecation here that is off by default but can be turned on with a config flag?
* Separate slot setter and getter method
This introduces an explicit setter for slots in the form of
`with_#{slot_name}`. This makes it more clear when slots are being set
vs accessed but also allows chaining when slots have no arguments.
Before this change, the following would not work:
```ruby
component.header.with_content("my header!")
```
This fails due to `header` being passed no arguments and no block,
resulting in it acting as a getter, returning `nil` since the slot was
not defined. This is due to assuming that slots will always be passed
either a block or arguments and predates the `with_content` method.
Now, the above can be implemented using `with_*`:
```ruby
component.with_header.with_content("my header!")
```
* Fix linter, add changelog entry
* Update lib/view_component/slotable_v2.rb
Co-authored-by: Hans Lemuet <Spone@users.noreply.github.com>
* Update lib/view_component/slotable_v2.rb
* Update docs/CHANGELOG.md
* Update docs, support polymorphic slots, add tests
* Fix ruby2_keywords call
Co-authored-by: Hans Lemuet <Spone@users.noreply.github.com>
Co-authored-by: Joel Hawksley <joel@hawksley.org>
* Separate slot setter and getter method
This introduces an explicit setter for slots in the form of
`with_#{slot_name}`. This makes it more clear when slots are being set
vs accessed but also allows chaining when slots have no arguments.
Before this change, the following would not work:
```ruby
component.header.with_content("my header!")
```
This fails due to `header` being passed no arguments and no block,
resulting in it acting as a getter, returning `nil` since the slot was
not defined. This is due to assuming that slots will always be passed
either a block or arguments and predates the `with_content` method.
Now, the above can be implemented using `with_*`:
```ruby
component.with_header.with_content("my header!")
```
* Fix linter, add changelog entry
* Update lib/view_component/slotable_v2.rb
Co-authored-by: Hans Lemuet <Spone@users.noreply.github.com>
* Update lib/view_component/slotable_v2.rb
* Update docs/CHANGELOG.md
* Update docs, support polymorphic slots, add tests
* Fix ruby2_keywords call
Co-authored-by: Hans Lemuet <Spone@users.noreply.github.com>
Co-authored-by: Joel Hawksley <joel@hawksley.org>
This introduces an explicit setter for slots in the form of
with_#{slot_name}. This makes it more clear when slots are being setvs accessed but also allows chaining when slots have no arguments.
Before this change, the following would not work:
This fails due to
headerbeing passed no arguments and no block,resulting in it acting as a getter, returning
nilsince the slot wasnot defined. This is due to the framework assuming that slots will always
be passed either a block or arguments and predates the
with_contentmethod.
Now, the above can be implemented using
with_*: