diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index f6830bb2b..599b1f823 100644 --- a/docs/CHANGELOG.md +++ b/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* diff --git a/docs/api.md b/docs/api.md index b2b3f3304..a4a820abe 100644 --- a/docs/api.md +++ b/docs/api.md @@ -103,14 +103,29 @@ 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. + +### `#render_parent_to_string` + +Renders the parent component to a string and returns it. This method is meant +to be used inside custom #call methods when a string result is desired, eg. + +```ruby +def call + "
#{render_parent_to_string}
" +end +``` + +When rendering the parent inside an .erb template, use `#render_parent` instead. ### `#request` → [ActionDispatch::Request] diff --git a/docs/guide/templates.md b/docs/guide/templates.md index 073191308..312d192cf 100644 --- a/docs/guide/templates.md +++ b/docs/guide/templates.md @@ -120,15 +120,54 @@ 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 %>
- <% render_parent %> + <%= render_parent %>
``` +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 +
+ <%= render_parent %> +
+ ERB +end +``` + +Finally, `#render_parent` also works inside `#call` methods: + +```ruby +class MyComponent < ViewComponent::Base + def call + content_tag("div") do + render_parent + end + end +end +``` + +When composing `#call` methods, keep in mind that `#render_parent` does not return a string. If a string is desired, call `#render_parent_to_string` instead. For example: + +```ruby +class MyComponent < ViewComponent::Base + # "phone" variant + def call_phone + content_tag("div") do + "
#{render_parent_to_string}
" + end + end +end +``` + ## 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. diff --git a/lib/view_component/base.rb b/lib/view_component/base.rb index 0f2910b8b..ac3256a3c 100644 --- a/lib/view_component/base.rb +++ b/lib/view_component/base.rb @@ -116,18 +116,42 @@ 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 + + # Renders the parent component to a string and returns it. This method is meant + # to be used inside custom #call methods when a string result is desired, eg. + # + # ```ruby + # def call + # "
#{render_parent_to_string}
" + # end + # ``` + # + # When rendering the parent inside an .erb template, use `#render_parent` instead. + def render_parent_to_string + capture { render_parent } end # Optional content to be returned after the rendered template. @@ -459,6 +483,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 diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index cfac92617..72c7367c4 100644 --- a/lib/view_component/compiler.rb +++ b/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.underscore.gsub("/", "__") + 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 diff --git a/test/sandbox/app/.DS_Store b/test/sandbox/app/.DS_Store new file mode 100644 index 000000000..48333a58e Binary files /dev/null and b/test/sandbox/app/.DS_Store differ diff --git a/test/sandbox/app/components/inline_level1_component.rb b/test/sandbox/app/components/inline_level1_component.rb new file mode 100644 index 000000000..bbc996d85 --- /dev/null +++ b/test/sandbox/app/components/inline_level1_component.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class InlineLevel1Component < ViewComponent::Base + def call + content_tag(:div, class: "level1-component") + end +end diff --git a/test/sandbox/app/components/inline_level2_component.rb b/test/sandbox/app/components/inline_level2_component.rb new file mode 100644 index 000000000..bb6378589 --- /dev/null +++ b/test/sandbox/app/components/inline_level2_component.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class InlineLevel2Component < Level2Component + def call + "
#{render_parent_to_string}
" + end + + def call_variant + "
#{render_parent_to_string}
" + end +end diff --git a/test/sandbox/app/components/inline_level3_component.rb b/test/sandbox/app/components/inline_level3_component.rb new file mode 100644 index 000000000..99e0c4aff --- /dev/null +++ b/test/sandbox/app/components/inline_level3_component.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class InlineLevel3Component < Level2Component + def call + content_tag(:div, class: "level3-component base") do + render_parent + end + end + + def call_variant + content_tag(:div, class: "level3-component variant") do + render_parent + end + end +end diff --git a/test/sandbox/app/components/level1_component.html.erb b/test/sandbox/app/components/level1_component.html.erb new file mode 100644 index 000000000..4eaf6c9b1 --- /dev/null +++ b/test/sandbox/app/components/level1_component.html.erb @@ -0,0 +1 @@ +
diff --git a/test/sandbox/app/components/level1_component.rb b/test/sandbox/app/components/level1_component.rb new file mode 100644 index 000000000..a47e7850c --- /dev/null +++ b/test/sandbox/app/components/level1_component.rb @@ -0,0 +1,4 @@ +# frozen_string_literal: true + +class Level1Component < ViewComponent::Base +end diff --git a/test/sandbox/app/components/level2_component.html+variant.erb b/test/sandbox/app/components/level2_component.html+variant.erb new file mode 100644 index 000000000..ca3e8fe7b --- /dev/null +++ b/test/sandbox/app/components/level2_component.html+variant.erb @@ -0,0 +1,3 @@ +
+ <%= render_parent %> +
diff --git a/test/sandbox/app/components/level2_component.html.erb b/test/sandbox/app/components/level2_component.html.erb new file mode 100644 index 000000000..ec3815262 --- /dev/null +++ b/test/sandbox/app/components/level2_component.html.erb @@ -0,0 +1,3 @@ +
+ <%= render_parent %> +
diff --git a/test/sandbox/app/components/level2_component.rb b/test/sandbox/app/components/level2_component.rb new file mode 100644 index 000000000..f8d7973ab --- /dev/null +++ b/test/sandbox/app/components/level2_component.rb @@ -0,0 +1,4 @@ +# frozen_string_literal: true + +class Level2Component < Level1Component +end diff --git a/test/sandbox/app/components/level3_component.html+variant.erb b/test/sandbox/app/components/level3_component.html+variant.erb new file mode 100644 index 000000000..dce97454a --- /dev/null +++ b/test/sandbox/app/components/level3_component.html+variant.erb @@ -0,0 +1,3 @@ +
+ <%= render_parent %> +
diff --git a/test/sandbox/app/components/level3_component.html.erb b/test/sandbox/app/components/level3_component.html.erb new file mode 100644 index 000000000..e9621744f --- /dev/null +++ b/test/sandbox/app/components/level3_component.html.erb @@ -0,0 +1,3 @@ +
+ <%= render_parent %> +
diff --git a/test/sandbox/app/components/level3_component.rb b/test/sandbox/app/components/level3_component.rb new file mode 100644 index 000000000..c34c13327 --- /dev/null +++ b/test/sandbox/app/components/level3_component.rb @@ -0,0 +1,4 @@ +# frozen_string_literal: true + +class Level3Component < Level2Component +end diff --git a/test/sandbox/test/inline_template_test.rb b/test/sandbox/test/inline_template_test.rb index 76ce0877b..986db8b6b 100644 --- a/test/sandbox/test/inline_template_test.rb +++ b/test/sandbox/test/inline_template_test.rb @@ -30,6 +30,9 @@ def initialize(name) class InlineErbSubclassComponent < InlineErbComponent erb_template <<~ERB

Hey, <%= name %>!

+
+ <%= render_parent %> +
ERB end @@ -76,6 +79,14 @@ def initialize(name) end end + class InlineComponentDerivedFromComponentSupportingVariants < Level2Component + erb_template <<~ERB +
+ <%= render_parent %> +
+ 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 diff --git a/test/sandbox/test/rendering_test.rb b/test/sandbox/test/rendering_test.rb index 89008eddf..f9bac0557 100644 --- a/test/sandbox/test/rendering_test.rb +++ b/test/sandbox/test/rendering_test.rb @@ -960,15 +960,59 @@ 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_child_components_can_render_parent_with_inline_templates + render_inline(InlineLevel3Component.new) + + assert_selector(".level3-component.base .level2-component.base .level1-component") + end + + def test_variant_propagates_to_parent_with_inline_templates + with_variant :variant do + render_inline(InlineLevel3Component.new) + end + + assert_selector ".level3-component.variant .level2-component.variant .level1-component" + end + + def test_child_components_fall_back_to_default_variant_with_inline_templates + with_variant :non_existent_variant do + render_inline(InlineLevel3Component.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"