Fix render_parent when variants are involved

Gemfile.lock

@@ -329,6 +329,9 @@ DEPENDENCIES
   jbuilder (~> 2)
   m (~> 1)
   minitest (~> 5.18)
+  net-imap
+  net-pop
+  net-smtp
   pry (~> 0.13)
   puma (~> 6)
   rails (~> 7.0.0)

docs/CHANGELOG.md

@@ -14,6 +14,10 @@ nav_order: 5
 
     *Chris Nitsas*
 
+* Introduce `yield :parent` as a replacement for `#render_parent`, which respects variants and deep inheritance hierarchies.
+
+    *Cameron Dutro*
+
 ## 3.4.0
 
 * Avoid including Rails `url_helpers` into `Preview` class when they're not defined.

docs/api.md

@@ -102,6 +102,8 @@ Returns HTML that has been escaped by the respective template handler.
 
 ### `#render_parent`
 
+DEPRECATED
+
 Subclass components that call `super` inside their template code will cause a
 double render if they emit the result:
 

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,18 @@ 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
     # ```
     #
-    # Calls `super`, returning `nil` to avoid rendering the result twice.
+    # `super` also does not consider the current variant. `render_parent` penders 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_call_block&.call
     end
 
     # Optional content to be returned after the rendered template.

lib/view_component/compiler.rb

@@ -46,16 +46,15 @@ def compile(raise_errors: false, force: false)
       if has_inline_template?
         template = component_class.inline_template
 
-        redefinition_lock.synchronize do
-          component_class.silence_redefinition_of_method("call")
-          # rubocop:disable Style/EvalWithLocation
-          component_class.class_eval <<-RUBY, template.path, template.lineno
-          def call
-            #{compiled_inline_template(template)}
-          end
-          RUBY
-          # rubocop:enable Style/EvalWithLocation
+        template_info = {
+          path: template.path,
+          lineno: template.lineno - 4,
+          body: compiled_inline_template(template)
+        }
+
+        define_compiled_template_methods("call", template_info)
 
+        redefinition_lock.synchronize do
           component_class.silence_redefinition_of_method("render_template_for")
           component_class.class_eval <<-RUBY, __FILE__, __LINE__ + 1
           def render_template_for(variant = nil)
@@ -65,20 +64,14 @@ def render_template_for(variant = nil)
         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])
+          template_info = {
+            path: template[:path],
+            lineno: -4,
+            body: compiled_template(template[:path])
+          }
 
-          redefinition_lock.synchronize do
-            component_class.silence_redefinition_of_method(method_name)
-            # rubocop:disable Style/EvalWithLocation
-            component_class.class_eval <<-RUBY, template[:path], 0
-            def #{method_name}
-              #{compiled_template(template[:path])}
-            end
-            RUBY
-            # rubocop:enable Style/EvalWithLocation
-          end
+          define_compiled_template_methods(method_name, template_info)
         end
 
         define_render_template_for
@@ -93,6 +86,54 @@ def #{method_name}
 
     attr_reader :component_class, :redefinition_lock
 
+    def define_compiled_template_methods(method_name, template_info)
+      unique_method_name = "#{method_name}__#{methodize(component_class.name)}"
+      unique_superclass_name = methodize(component_class.superclass.name)
+
+      redefinition_lock.synchronize do
+        # Remove existing compiled template methods,
+        # as Ruby warns when redefining a method.
+        component_class.silence_redefinition_of_method(method_name)
+        component_class.silence_redefinition_of_method(unique_method_name)
+
+        # rubocop:disable Style/EvalWithLocation
+        component_class.class_eval <<-RUBY, template_info[:path], template_info[:lineno]
+        private def #{unique_method_name}(&block)
+          if block
+            @__vc_parent_call_block = block
+
+            begin
+              #{template_info[:body]}
+            end.tap do
+              @__vc_parent_call_block = nil
+            end
+          else
+            #{unique_method_name} do
+              super_method_name = if @__vc_variant
+                super_variant_method_name = :"call_\#{@__vc_variant}__#{unique_superclass_name}"
+                respond_to?(super_variant_method_name, true) ? super_variant_method_name : nil
+              end
+
+              super_method_name ||= :call__#{unique_superclass_name}
+              send(super_method_name)
+
+              nil
+            end
+          end
+        end
+
+        def #{method_name}
+          #{unique_method_name}
+        end
+        RUBY
+        # rubocop:enable Style/EvalWithLocation
+      end
+    end
+
+    def methodize(str)
+      str.gsub("::", "_").underscore
+    end
+
     def define_render_template_for
       variant_elsifs = variants.compact.uniq.map do |variant|
         "elsif variant.to_sym == :'#{variant}'\n    #{call_method_name(variant)}"
@@ -277,11 +318,12 @@ def normalized_variant_name(variant)
     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">
+        <%= yield :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"