Fix render_parent when variants are involved

docs/CHANGELOG.md

@@ -14,6 +14,10 @@ nav_order: 5
 
     *Chris Nitsas*
 
+* Improve implementation of `#render_parent` so it respects variants and deep inheritance hierarchies.
+
+    *Cameron Dutro*
+
 * Add CharlieHR to users list.
 
     *Alex Balhatchet*

docs/api.md

@@ -103,14 +103,16 @@ Returns HTML that has been escaped by the respective template handler.
 ### `#render_parent`
 
 Subclass components that call `super` inside their template code will cause a
-double render if they emit the result:
+double render if they emit the result.
 
 ```erb
 <%= super %> # double-renders
-<% super %> # does not double-render
+<% super %> # doesn't double-render
 ```
 
-Calls `super`, returning `nil` to avoid rendering the result twice.
+`super` also doesn't consider the current variant. `render_parent` renders the
+parent template considering the current variant and emits the result without
+double-rendering.
 
 ### `#request` → [ActionDispatch::Request]
 

docs/guide/templates.md

@@ -120,15 +120,42 @@ end
 Since 2.55.0
 {: .label }
 
-To render a parent component's template from a subclass, call `render_parent`:
+To render a parent component's template from a subclass' template, use `#render_parent`:
 
 ```erb
 <%# my_link_component.html.erb %>
 <div class="base-component-template">
-  <% render_parent %>
+  <%= render_parent %>
 </div>
 ```
 
+If the parent supports the current variant, the variant will automatically be rendered.
+
+`#render_parent` also works with inline templates:
+
+```ruby
+class MyComponent < ViewComponent::Base
+  erb_template <<~ERB
+    <div>
+      <%= render_parent %>
+    </div>
+  ERB
+end
+```
+
+To render a parent component's template from a `#call` method, call `super`.
+
+```ruby
+class MyComponent < ViewComponent::Base
+  # "phone" variant
+  def call_phone
+    "<div>#{super}</div>"
+  end
+end
+```
+
+`super` will attempt to call the `#call_phone` method on the parent class. If the parent class doesn't support the "phone" variant, Ruby will raise a `NoMethodError`. Consider using a template and `#render_parent` to handle superclass variants automatically.
+
 ## Trailing whitespace
 
 Code editors commonly add a trailing newline character to source files in keeping with the Unix standard. Including trailing whitespace in component templates can result in unwanted whitespace in the HTML, eg. if the component is rendered before the period at the end of a sentence.

lib/view_component/base.rb

@@ -116,18 +116,28 @@ def render_in(view_context, &block)
     end
 
     # Subclass components that call `super` inside their template code will cause a
-    # double render if they emit the result:
+    # double render if they emit the result.
     #
     # ```erb
     # <%= super %> # double-renders
-    # <% super %> # does not double-render
+    # <% super %> # doesn't double-render
     # ```
     #
-    # Calls `super`, returning `nil` to avoid rendering the result twice.
+    # `super` also doesn't consider the current variant. `render_parent` renders the
+    # parent template considering the current variant and emits the result without
+    # double-rendering.
     def render_parent
-      mtd = @__vc_variant ? "call_#{@__vc_variant}" : "call"
-      method(mtd).super_method.call
-      nil
+      @__vc_parent_render_level ||= 0 # ensure a good starting value
+
+      begin
+        target_render = self.class.instance_variable_get(:@__vc_ancestor_calls)[@__vc_parent_render_level]
+        @__vc_parent_render_level += 1
+
+        target_render.bind_call(self, @__vc_variant)
+        nil
+      ensure
+        @__vc_parent_render_level -= 1
+      end
     end
 
     # Optional content to be returned after the rendered template.
@@ -459,6 +469,13 @@ def render_template_for(variant = nil)
         # Set collection parameter to the extended component
         child.with_collection_parameter provided_collection_parameter
 
+        if instance_methods(false).include?(:render_template_for)
+          vc_ancestor_calls = defined?(@__vc_ancestor_calls) ? @__vc_ancestor_calls.dup : []
+
+          vc_ancestor_calls.unshift(instance_method(:render_template_for))
+          child.instance_variable_set(:@__vc_ancestor_calls, vc_ancestor_calls)
+        end
+
         super
       end
 

lib/view_component/compiler.rb

@@ -56,17 +56,17 @@ def call
           RUBY
           # rubocop:enable Style/EvalWithLocation
 
+          component_class.define_method("_call_#{safe_class_name}", component_class.instance_method(:call))
+
           component_class.silence_redefinition_of_method("render_template_for")
           component_class.class_eval <<-RUBY, __FILE__, __LINE__ + 1
           def render_template_for(variant = nil)
-            call
+            _call_#{safe_class_name}
           end
           RUBY
         end
       else
         templates.each do |template|
-          # Remove existing compiled template methods,
-          # as Ruby warns when redefining a method.
           method_name = call_method_name(template[:variant])
 
           redefinition_lock.synchronize do
@@ -95,15 +95,20 @@ def #{method_name}
 
     def define_render_template_for
       variant_elsifs = variants.compact.uniq.map do |variant|
-        "elsif variant.to_sym == :'#{variant}'\n    #{call_method_name(variant)}"
+        safe_name = "_call_variant_#{normalized_variant_name(variant)}_#{safe_class_name}"
+        component_class.define_method(safe_name, component_class.instance_method(call_method_name(variant)))
+
+        "elsif variant.to_sym == :'#{variant}'\n    #{safe_name}"
       end.join("\n")
 
+      component_class.define_method("_call_#{safe_class_name}", component_class.instance_method(:call))
+
       body = <<-RUBY
         if variant.nil?
-          call
+          _call_#{safe_class_name}
         #{variant_elsifs}
         else
-          call
+          _call_#{safe_class_name}
         end
       RUBY
 
@@ -276,12 +281,17 @@ def normalized_variant_name(variant)
       variant.to_s.gsub("-", "__").gsub(".", "___")
     end
 
+    def safe_class_name
+      @safe_class_name ||= component_class.name.gsub("::", "_").underscore
+    end
+
     def should_compile_superclass?
-      development? && templates.empty? && !has_inline_template? &&
-        !(
-          component_class.instance_methods(false).include?(:call) ||
-            component_class.private_instance_methods(false).include?(:call)
-        )
+      development? && templates.empty? && !has_inline_template? && !call_defined?
+    end
+
+    def call_defined?
+      component_class.instance_methods(false).include?(:call) ||
+        component_class.private_instance_methods(false).include?(:call)
     end
   end
 end

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

@@ -0,0 +1 @@
+<div class="level1-component"></div>

test/sandbox/app/components/level1_component.rb

@@ -0,0 +1,4 @@
+# frozen_string_literal: true
+
+class Level1Component < ViewComponent::Base
+end

test/sandbox/app/components/level2_component.html+variant.erb

@@ -0,0 +1,3 @@
+<div class="level2-component variant">
+  <%= render_parent %>
+</div>

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

@@ -0,0 +1,3 @@
+<div class="level2-component base">
+  <%= render_parent %>
+</div>

test/sandbox/app/components/level2_component.rb

@@ -0,0 +1,4 @@
+# frozen_string_literal: true
+
+class Level2Component < Level1Component
+end

test/sandbox/app/components/level3_component.html+variant.erb

@@ -0,0 +1,3 @@
+<div class="level3-component variant">
+  <%= render_parent %>
+</div>

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

@@ -0,0 +1,3 @@
+<div class="level3-component base">
+  <%= render_parent %>
+</div>

test/sandbox/app/components/level3_component.rb

@@ -0,0 +1,4 @@
+# frozen_string_literal: true
+
+class Level3Component < Level2Component
+end

test/sandbox/test/inline_template_test.rb

@@ -30,6 +30,9 @@ def initialize(name)
   class InlineErbSubclassComponent < InlineErbComponent
     erb_template <<~ERB
       <h1>Hey, <%= name %>!</h1>
+      <div class="parent">
+        <%= render_parent %>
+      </div>
     ERB
   end
 
@@ -76,6 +79,14 @@ def initialize(name)
     end
   end
 
+  class InlineComponentDerivedFromComponentSupportingVariants < Level2Component
+    erb_template <<~ERB
+      <div class="inline-template">
+        <%= render_parent %>
+      </div>
+    ERB
+  end
+
   test "renders inline templates" do
     render_inline(InlineErbComponent.new("Fox Mulder"))
 
@@ -112,6 +123,20 @@ def initialize(name)
     assert_selector("h1", text: "Hey, Fox Mulder!")
   end
 
+  test "child components can render their parent" do
+    render_inline(InlineErbSubclassComponent.new("Fox Mulder"))
+
+    assert_selector(".parent h1", text: "Hello, Fox Mulder!")
+  end
+
+  test "inline child component propagates variant to parent" do
+    with_variant :variant do
+      render_inline(InlineComponentDerivedFromComponentSupportingVariants.new)
+    end
+
+    assert_selector ".inline-template .level2-component.variant .level1-component"
+  end
+
   test "calling template methods multiple times raises an exception" do
     error = assert_raises ViewComponent::MultipleInlineTemplatesError do
       Class.new(InlineErbComponent) do

test/sandbox/test/rendering_test.rb

@@ -960,15 +960,37 @@ def test_inherited_component_renders_when_lazy_loading
     assert_selector("div", text: "hello, my own template")
   end
 
-  def test_inherited_component_calls_super
+  def test_render_parent
     render_inline(SuperComponent.new)
 
     assert_selector(".base-component", count: 1)
-    assert_selector(".derived-component", count: 1) do
-      assert_selector(".base-component", count: 1)
+    assert_selector(".derived-component", count: 1) do |derived|
+      derived.assert_selector(".base-component", count: 1)
     end
   end
 
+  def test_child_components_can_render_parent
+    render_inline(Level3Component.new)
+
+    assert_selector(".level3-component.base .level2-component.base .level1-component")
+  end
+
+  def test_variant_propagates_to_parent
+    with_variant :variant do
+      render_inline(Level3Component.new)
+    end
+
+    assert_selector ".level3-component.variant .level2-component.variant .level1-component"
+  end
+
+  def test_child_components_fall_back_to_default_variant
+    with_variant :non_existent_variant do
+      render_inline(Level3Component.new)
+    end
+
+    assert_selector ".level3-component.base .level2-component.base .level1-component"
+  end
+
   def test_component_renders_without_trailing_whitespace
     template = File.read(Rails.root.join("app/components/trailing_whitespace_component.html.erb"))
     assert template =~ /\s+\z/, "Template does not contain any trailing whitespace"