Add support for polymorphic slots (#1083)

.github/workflows/ci.yml

@@ -167,7 +167,7 @@ jobs:
         cd primer_view_components
         yarn install
         bundle config path vendor/bundle
-        bundle update
+        bundle install
         bundle exec rake docs:preview
         bundle exec rake
       env:

.tool-versions

@@ -0,0 +1 @@
+ruby 3.0.2

adr/0003-polymorphic-slot-definitions.md

@@ -0,0 +1,120 @@
+# 3. Polymorphic slots
+
+Date: 2021-09-29
+
+## Author
+
+Cameron Dutro
+
+## Status
+
+Accepted
+
+## Context
+
+Components can currently define slots in two ways:
+
+1. by specifying a component class (or class name string)
+1. by providing a proc (i.e. lambda) that either returns HTML or a component instance.
+
+With these options in mind, imagine a scenario in which a component supports rendering one of two possible sub-components in a slot. In other words, the user of the component may only fill the slot with one of two (or more) possible kinds of sub-component.
+
+To illustrate, let's consider a list component with an `items` slot. Each constituent `Item` has either an icon or an avatar on the right-hand side followed by some text.
+
+When implementing the `Item` component, we have several options for determining whether we should render an icon or an avatar:
+
+1. **Two slots w/error**: define two different slots for the icon and avatar, and raise an error in the `before_render` lifecycle method if both are defined.
+1. **Two slots w/default**: define two different slots for the icon and avatar, but favor one or the other if both are provided.
+1. **Examine kwargs**: define a single slot and determine which sub-component to render by examining the contents of `**kwargs`.
+1. **Unrestricted content**: define a single slot that renders any content provided by the caller. The component has to "trust" that the caller will pass in only an icon or avatar.
+
+While all of these options are reasonably acceptable, there are problems with each:
+
+1. **Two slots w/error**: using `before_render` for slot validation feels like an anti-pattern. To make the interface clear, defining both slots shouldn't be possible.
+1. **Two slots w/default**: same issues as #1, but worse because it silently "swallows" the error. This behavior probably won't be obvious to the component's users.
+1. **Examine kwargs**: this approach is brittle because the kwargs accepted by constituent components can change over time, potentially requiring changes to the `Item` component as well.
+1. **Unrestricted content**: not ideal because the content can literally be anything and relies on the caller following the "rules."
+
+It is my opinion that we need the ability to choose between multiple types within a single slot.
+
+## Decision
+
+We will introduce a third type of slot called a polymorphic slot. The `renders_one` and `renders_many` methods will accept a mapping of the various acceptable sub-components. Each of these sub-components will themselves be slot definitions, meaning they can be defined as either a class/string or proc.
+
+Here's how the `Item` sub-component of the list example above would be implemented using polymorphic slots:
+
+```ruby
+class Item < ViewComponent::Base
+  include ViewComponent::PolymorphicSlots
+
+  renders_one :leading_visual, types: {
+    icon: IconComponent, avatar: AvatarComponent
+  }
+end
+```
+
+The `Item` component can then be used like this:
+
+```html+erb
+<%= render List.new do |component| %>
+  <% component.item do |item| %>
+    <% item.leading_visual_avatar(src: "assets/user/1234.png") %>
+    Profile
+  <% end %>
+  <% component.item do |item| %>
+    <% item.leading_visual_icon(icon: :gear) %>
+    Settings
+  <% end %>
+<% end %>
+```
+
+Notice that the type of leading visual, either `:icon` or `:avatar`, is appended to the slot name, `leading_visual`, and corresponds to the items in the `types` hash passed to `renders_one`.
+
+Finally, the polymorphic slot behavior will be implemented as a `module` so the behavior is opt-in until we're confident that it's a good addition to ViewComponent.
+
+## Consequences
+
+Things we tried and things we've learned.
+
+### Additional Complexity
+
+The biggest consequence of this design is that it makes the slots API more complicated, something the view_component maintainers have been hesitant to do given the confusion we routinely see around slots.
+
+### Content Wrapping
+
+One concern of the proposed approach is that it offers no immediately obvious way to wrap the contents of a slot. Here's an example of how a slot might be wrapped:
+
+```ruby
+renders_many :items do |*args, **kwargs|
+  content_tag :td, class: kwargs[:table_row_classes] do
+    Row.new(*args, **kwargs)
+  end
+end
+```
+
+In such cases, there are several viable workarounds:
+
+1. Add the wrapping HTML to the template.
+1. Provide a lambda for each polymorphic type that adds the wrapping HTML. There is the potential for code duplication here, which could be mitigated by calling a class or helper method.
+1. Manually implement a polymorphic slot using a positional `type` argument and `case` statement, as shown in the example below. This effectively replicates the behavior described in this proposal.
+    ```ruby
+    renders_many :items do |type, *args, **kwargs|
+      content_tag :td, class: kwargs[:table_row_classes] do
+        case type
+          when :foo
+            RowFoo.new(*args, **kwargs)
+          when :bar
+            RowBar.new(*args, **kwargs)
+        end
+      end
+    end
+    ```
+
+### Positional Type Argument vs Method Names
+
+There has been some discussion around whether or not polymorphic slots should accept a positional `type` argument or instead define methods that correspond to each slot type as described in this ADR. We have decided to implement the method approach for several reasons:
+
+1. Positional arguments aren't used anywhere else in the framework.
+1. There is a preference amongst team members that the slot setter accept the exact same arguments as the slot itself, since doing so reduces the conceptual overhead of the slots API.
+
+An argument was made that multiple setters for the same slot appear to be two different slots, but wasn't considered enough of a drawback to go the `type` argument route.

docs/CHANGELOG.md

@@ -23,6 +23,10 @@ title: Changelog
 
     *Blake Williams*, *Ian C. Anderson*
 
+* Implement polymorphic slots as experimental feature. See the Slots documentation to learn more.
+
+    *Cameron Dutro*
+
 ## 2.41.0
 
 * Add `sprockets-rails` development dependency to fix test suite failures when using rails@main.

docs/api.md

@@ -49,6 +49,14 @@ that inhibits encapsulation & reuse, often making testing difficult.
 A proxy through which to access helpers. Use sparingly as doing so introduces
 coupling that inhibits encapsulation & reuse, often making testing difficult.
 
+### #original_view_context
+
+Returns the value of attribute original_view_context.
+
+### #original_view_context=(value)
+
+Sets the attribute original_view_context
+
 ### #render? → [Boolean]
 
 Override to determine whether the ViewComponent should render.

docs/guide/slots.md

@@ -186,3 +186,39 @@ Slot content can also be set using `#with_content`:
 ```
 
 _To view documentation for content_areas (deprecated) and the original implementation of Slots (deprecated), see [/content_areas](/content_areas) and [/slots_v1](/slots_v1)._
+
+## Polymorphic slots (Experimental)
+
+Polymorphic slots can render one of several possible slots. To use this experimental feature, include `ViewComponent::PolymorphicSlots`.
+
+For example, consider this list item component that can be rendered with either an icon or an avatar visual. The `visual` slot is passed a hash mapping types to slot definitions:
+
+```ruby
+class ListItemComponent < ViewComponent::Base
+  include ViewComponent::PolymorphicSlots
+
+  renders_one :visual, types: {
+    icon: IconComponent,
+    avatar: lambda { |**system_arguments|
+      AvatarComponent.new(size: 16, **system_arguments)
+    }
+  }
+end
+```
+
+**NOTE**: the `types` hash's values can be any valid slot definition, including a component class, string, or lambda.
+
+Filling in the `visual` slot is done by calling the appropriate slot method:
+
+```erb
+<%= render ListItemComponent.new do |c| %>
+  <% c.visual_avatar(src: "http://some-site.com/my_avatar.jpg", alt: "username") %>
+    Profile
+  <% end >
+<% end %>
+<%= render ListItemComponent.new do |c| %>
+  <% c.visual_icon(icon: :key) %>
+    Security Settings
+  <% end >
+<% end %>
+```

lib/view_component/base.rb

@@ -5,6 +5,7 @@ require "active_support/configurable"
 require "view_component/collection"
 require "view_component/compile_cache"
 require "view_component/content_areas"
+require "view_component/polymorphic_slots"
 require "view_component/previewable"
 require "view_component/slotable"
 require "view_component/slotable_v2"

lib/view_component/polymorphic_slots.rb

@@ -0,0 +1,74 @@
+# frozen_string_literal: true
+
+module ViewComponent
+  module PolymorphicSlots
+    # In older rails versions, using a concern isn't a good idea here because they appear to not work with
+    # Module#prepend and class methods.
+    def self.included(base)
+      base.singleton_class.prepend(ClassMethods)
+      base.include(InstanceMethods)
+    end
+
+    module ClassMethods
+      def renders_one(slot_name, callable = nil)
+        return super unless callable.is_a?(Hash) && callable.key?(:types)
+
+        validate_singular_slot_name(slot_name)
+        register_polymorphic_slot(slot_name, callable[:types], collection: false)
+      end
+
+      def renders_many(slot_name, callable = nil)
+        return super unless callable.is_a?(Hash) && callable.key?(:types)
+
+        validate_plural_slot_name(slot_name)
+        register_polymorphic_slot(slot_name, callable[:types], collection: true)
+      end
+
+      def register_polymorphic_slot(slot_name, types, collection:)
+        renderable_hash = types.each_with_object({}) do |(poly_type, poly_callable), memo|
+          memo[poly_type] = define_slot(
+            "#{slot_name}_#{poly_type}", collection: collection, callable: poly_callable
+          )
+
+          getter_name = slot_name
+          setter_name =
+            if collection
+              "#{ActiveSupport::Inflector.singularize(slot_name)}_#{poly_type}"
+            else
+              "#{slot_name}_#{poly_type}"
+            end
+
+          define_method(getter_name) do
+            get_slot(slot_name)
+          end
+          ruby2_keywords(getter_name.to_sym) if respond_to?(:ruby2_keywords, true)
+
+          define_method(setter_name) do |*args, &block|
+            set_polymorphic_slot(slot_name, poly_type, *args, &block)
+          end
+          ruby2_keywords(setter_name.to_sym) if respond_to?(:ruby2_keywords, true)
+        end
+
+        self.registered_slots[slot_name] = {
+          collection: collection,
+          renderable_hash: renderable_hash
+        }
+      end
+    end
+
+    module InstanceMethods
+      def set_polymorphic_slot(slot_name, poly_type = nil, *args, &block)
+        slot_definition = self.class.registered_slots[slot_name]
+
+        if !slot_definition[:collection] && get_slot(slot_name)
+          raise ArgumentError, "content for slot '#{slot_name}' has already been provided"
+        end
+
+        poly_def = slot_definition[:renderable_hash][poly_type]
+
+        set_slot(slot_name, poly_def, *args, &block)
+      end
+      ruby2_keywords(:set_polymorphic_slot) if respond_to?(:ruby2_keywords, true)
+    end
+  end
+end

lib/view_component/slotable_v2.rb

@@ -1,7 +1,6 @@
 # frozen_string_literal: true
 
 require "active_support/concern"
-
 require "view_component/slot_v2"
 
 module ViewComponent
@@ -71,7 +70,7 @@ module ViewComponent
           if args.empty? && block.nil?
             get_slot(slot_name)
           else
-            set_slot(slot_name, *args, &block)
+            set_slot(slot_name, nil, *args, &block)
           end
         end
         ruby2_keywords(slot_name.to_sym) if respond_to?(:ruby2_keywords, true)
@@ -125,7 +124,7 @@ module ViewComponent
         # e.g. `renders_many :items` allows fetching all tabs with
         # `component.tabs` and setting a tab with `component.tab`
         define_method singular_name do |*args, &block|
-          set_slot(slot_name, *args, &block)
+          set_slot(slot_name, nil, *args, &block)
         end
         ruby2_keywords(singular_name.to_sym) if respond_to?(:ruby2_keywords, true)
 
@@ -136,7 +135,7 @@ module ViewComponent
             get_slot(slot_name)
           else
             collection_args.map do |args|
-              set_slot(slot_name, **args, &block)
+              set_slot(slot_name, nil, **args, &block)
             end
           end
         end
@@ -164,27 +163,37 @@ module ViewComponent
 
       private
 
-      def register_slot(slot_name, collection:, callable:)
+      def register_slot(slot_name, **kwargs)
+        self.registered_slots[slot_name] = define_slot(slot_name, **kwargs)
+      end
+
+      def define_slot(slot_name, collection:, callable:)
         # Setup basic slot data
         slot = {
           collection: collection,
         }
+        return slot unless callable
+
         # If callable responds to `render_in`, we set it on the slot as a renderable
-        if callable && callable.respond_to?(:method_defined?) && callable.method_defined?(:render_in)
+        if callable.respond_to?(:method_defined?) && callable.method_defined?(:render_in)
           slot[:renderable] = callable
         elsif callable.is_a?(String)
           # If callable is a string, we assume it's referencing an internal class
           slot[:renderable_class_name] = callable
-        elsif callable
+        elsif callable.respond_to?(:call)
           # If slot does not respond to `render_in`, we assume it's a proc,
           # define a method, and save a reference to it to call when setting
           method_name = :"_call_#{slot_name}"
           define_method method_name, &callable
           slot[:renderable_function] = instance_method(method_name)
+        else
+          raise(
+            ArgumentError,
+            "invalid slot definition. Please pass a class, string, or callable (i.e. proc, lambda, etc)"
+          )
         end
 
-        # Register the slot on the component
-        self.registered_slots[slot_name] = slot
+        slot
       end
 
       def validate_plural_slot_name(slot_name)
@@ -237,9 +246,8 @@ module ViewComponent
       end
     end
 
-    def set_slot(slot_name, *args, &block)
-      slot_definition = self.class.registered_slots[slot_name]
-
+    def set_slot(slot_name, slot_definition = nil, *args, &block)
+      slot_definition ||= self.class.registered_slots[slot_name]
       slot = SlotV2.new(self)
 
       # Passing the block to the sub-component wrapper like this has two

test/sandbox/app/components/polymorphic_slot_component.html.erb

@@ -0,0 +1,6 @@
+<div>
+  <%= header %>
+  <% items.each do |item| %>
+    <%= item %>
+  <% end %>
+</div>

test/sandbox/app/components/polymorphic_slot_component.rb

@@ -0,0 +1,33 @@
+# frozen_string_literal: true
+
+class PolymorphicSlotComponent < ViewComponent::Base
+  include ViewComponent::PolymorphicSlots
+
+  renders_one :header, types: {
+    standard: lambda { |&block| content_tag(:div, class: "standard", &block) },
+    special: lambda { |&block| content_tag(:div, class: "special", &block) }
+  }
+
+  renders_many :items, types: {
+    foo: "FooItem",
+    bar: lambda { |class_names: "", **_system_arguments|
+      classes = (class_names.split(" ") + ["bar"]).join(" ")
+      content_tag(:div, class: classes) do
+        "bar item"
+      end
+    }
+  }
+
+  class FooItem < ViewComponent::Base
+    def initialize(class_names: "", **_system_arguments)
+      @class_names = class_names
+    end
+
+    def call
+      classes = (@class_names.split(" ") + ["foo"]).join(" ")
+      content_tag(:div, class: classes) do
+        "foo item"
+      end
+    end
+  end
+end

test/view_component/slotable_v2_test.rb

@@ -395,7 +395,7 @@ class SlotsV2sTest < ViewComponent::TestCase
     assert_includes error.message, "It looks like a block was provided after calling"
   end
 
-  def test_renders_lamda_slot_with_no_args
+  def test_renders_lambda_slot_with_no_args
     render_inline(SlotsV2WithEmptyLambdaComponent.new) do |c|
       c.item { "Item 1" }
       c.item { "Item 2" }
@@ -431,4 +431,45 @@ class SlotsV2sTest < ViewComponent::TestCase
   def test_slot_type_nil?
     assert_nil(SlotsV2Component.slot_type(:junk))
   end
+
+  def test_polymorphic_slot
+    render_inline(PolymorphicSlotComponent.new) do |component|
+      component.header_standard { "standard" }
+      component.item_foo(class_names: "custom-foo")
+      component.item_bar(class_names: "custom-bar")
+    end
+
+    assert_selector("div .standard", text: "standard")
+    assert_selector("div .foo.custom-foo:nth-child(2)")
+    assert_selector("div .bar.custom-bar:last")
+  end
+
+  def test_polymorphic_slot_non_member
+    assert_raises NoMethodError do
+      render_inline(PolymorphicSlotComponent.new) do |component|
+        component.item_non_existent
+      end
+    end
+  end
+
+  def test_singular_polymorphic_slot_raises_on_redefinition
+    error = assert_raises ArgumentError do
+      render_inline(PolymorphicSlotComponent.new) do |component|
+        component.header_standard { "standard" }
+        component.header_special { "special" }
+      end
+    end
+
+    assert_includes error.message, "has already been provided"
+  end
+
+  def test_invalid_slot_definition_raises_error
+    error = assert_raises ArgumentError do
+      Class.new(ViewComponent::Base) do
+        renders_many :items, :foo
+      end
+    end
+
+    assert_includes error.message, "invalid slot definition"
+  end
 end