diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 28362dc41..80fef2040 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -10,6 +10,10 @@ nav_order: 6 ## main +* Fix translation scope resolution in deeply nested component blocks (3+ levels). Translations called inside deeply nested slot blocks using `renders_many`/`renders_one` were incorrectly resolving to an intermediate component's scope instead of the partial's scope where the block was defined. The fix captures the virtual path at block definition time and restores it during block execution, ensuring translations always resolve relative to where the block was created regardless of nesting depth. Builds on #2389. Fixes #2386. + + *Nathaniel Watts* + * Allow `render_inline` with Nokogiri::HTML5 to parse more arbitrary content including bare table content otherwise illegal fragments like ``. *Jonathan Rochkind* diff --git a/lib/view_component/base.rb b/lib/view_component/base.rb index 68f15b244..c6d75acaa 100644 --- a/lib/view_component/base.rb +++ b/lib/view_component/base.rb @@ -363,7 +363,7 @@ def content @__vc_content = if __vc_render_in_block_provided? - with_original_virtual_path do + with_captured_virtual_path(@old_virtual_path) do view_context.capture(self, &@__vc_render_in_block) end elsif __vc_content_set_by_with_content_defined? @@ -379,11 +379,14 @@ def content? end # @private - def with_original_virtual_path - @view_context.instance_variable_set(:@virtual_path, @old_virtual_path) + # Temporarily sets the virtual path to the captured value, then restores it. + # This ensures translations and other path-dependent code execute with the correct scope. + def with_captured_virtual_path(captured_path) + old_virtual_path = @view_context.instance_variable_get(:@virtual_path) + @view_context.instance_variable_set(:@virtual_path, captured_path) yield ensure - @view_context.instance_variable_set(:@virtual_path, virtual_path) + @view_context.instance_variable_set(:@virtual_path, old_virtual_path) end private diff --git a/lib/view_component/slot.rb b/lib/view_component/slot.rb index 299ca7210..e70a421e2 100644 --- a/lib/view_component/slot.rb +++ b/lib/view_component/slot.rb @@ -6,7 +6,7 @@ module ViewComponent class Slot include ViewComponent::WithContentHelper - attr_writer :__vc_component_instance, :__vc_content_block, :__vc_content + attr_writer :__vc_component_instance, :__vc_content_block, :__vc_content, :__vc_content_block_virtual_path def initialize(parent) @parent = parent @@ -58,7 +58,7 @@ def to_s if defined?(@__vc_content_block) # render_in is faster than `parent.render` @__vc_component_instance.render_in(view_context) do |*args| - @parent.with_original_virtual_path do + @parent.with_captured_virtual_path(@__vc_content_block_virtual_path) do @__vc_content_block.call(*args) end end @@ -68,7 +68,7 @@ def to_s elsif defined?(@__vc_content) @__vc_content elsif defined?(@__vc_content_block) - @parent.with_original_virtual_path do + @parent.with_captured_virtual_path(@__vc_content_block_virtual_path) do view_context.capture(&@__vc_content_block) end elsif defined?(@__vc_content_set_by_with_content) diff --git a/lib/view_component/slotable.rb b/lib/view_component/slotable.rb index ac78b320e..03f745f20 100644 --- a/lib/view_component/slotable.rb +++ b/lib/view_component/slotable.rb @@ -390,7 +390,12 @@ def __vc_set_slot(slot_name, slot_definition = nil, *args, **kwargs, &block) # 2. Since we have to pass block content to components when calling # `render`, evaluating the block here would require us to call # `view_context.capture` twice, which is slower - slot.__vc_content_block = block if block + if block + slot.__vc_content_block = block + # Capture the virtual path at the time the block is defined, so that + # translations resolve relative to where the block was created, not where it's rendered + slot.__vc_content_block_virtual_path = view_context.instance_variable_get(:@virtual_path) + end # If class if slot_definition[:renderable] @@ -408,7 +413,9 @@ def __vc_set_slot(slot_name, slot_definition = nil, *args, **kwargs, &block) renderable_value = if block renderable_function.call(*args, **kwargs) do |*rargs| - view_context.capture(*rargs, &block) + with_captured_virtual_path(@old_virtual_path) do + view_context.capture(*rargs, &block) + end end else renderable_function.call(*args, **kwargs) diff --git a/test/sandbox/app/components/action_list_component.rb b/test/sandbox/app/components/action_list_component.rb new file mode 100644 index 000000000..eeabca655 --- /dev/null +++ b/test/sandbox/app/components/action_list_component.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +# Simplified ActionList component inspired by Primer::Alpha::ActionList +# Used to test deeply nested translation scoping +class ActionListComponent < ViewComponent::Base + renders_many :items, MenuItemComponent + + erb_template <<~ERB + + ERB +end diff --git a/test/sandbox/app/components/action_menu_component.rb b/test/sandbox/app/components/action_menu_component.rb new file mode 100644 index 000000000..5d90c88be --- /dev/null +++ b/test/sandbox/app/components/action_menu_component.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +# Simplified ActionMenu component inspired by Primer::Alpha::ActionMenu +# Used to test deeply nested translation scoping +class ActionMenuComponent < ViewComponent::Base + renders_one :list, ActionListComponent + + erb_template <<~ERB +
+ <%= list %> +
+ ERB +end diff --git a/test/sandbox/app/components/action_menu_panel_wrapper_component.rb b/test/sandbox/app/components/action_menu_panel_wrapper_component.rb new file mode 100644 index 000000000..a18d662ea --- /dev/null +++ b/test/sandbox/app/components/action_menu_panel_wrapper_component.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class ActionMenuPanelWrapperComponent < ViewComponent::Base + def call + render "shared/action_menu_panel" + end +end diff --git a/test/sandbox/app/components/deep_navigation_wrapper_component.rb b/test/sandbox/app/components/deep_navigation_wrapper_component.rb new file mode 100644 index 000000000..cf69fbd99 --- /dev/null +++ b/test/sandbox/app/components/deep_navigation_wrapper_component.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class DeepNavigationWrapperComponent < ViewComponent::Base + def call + render "shared/deep_navigation" + end +end diff --git a/test/sandbox/app/components/menu_item_component.rb b/test/sandbox/app/components/menu_item_component.rb new file mode 100644 index 000000000..49481b0b9 --- /dev/null +++ b/test/sandbox/app/components/menu_item_component.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +# Simplified MenuItem component inspired by Primer::Alpha::ActionList::Item +# Used to test deeply nested translation scoping +class MenuItemComponent < ViewComponent::Base + erb_template <<~ERB +
  • + <%= content %> +
    • + ERB +end diff --git a/test/sandbox/app/components/nav_component.rb b/test/sandbox/app/components/nav_component.rb new file mode 100644 index 000000000..0ae50bd89 --- /dev/null +++ b/test/sandbox/app/components/nav_component.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +# Level 2: Navigation component that contains an action menu +class NavComponent < ViewComponent::Base + renders_one :action_menu, ActionMenuComponent + + erb_template <<~ERB + + ERB +end diff --git a/test/sandbox/app/components/section_component.rb b/test/sandbox/app/components/section_component.rb new file mode 100644 index 000000000..efb76274e --- /dev/null +++ b/test/sandbox/app/components/section_component.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +# Level 0: Wraps a nav component (for 5-level deep testing) +class SectionComponent < ViewComponent::Base + renders_one :nav, NavComponent + + erb_template <<~ERB +
    + <%= nav %> +
    + ERB +end diff --git a/test/sandbox/app/views/shared/_action_menu_panel.html.erb b/test/sandbox/app/views/shared/_action_menu_panel.html.erb new file mode 100644 index 000000000..5e8d98862 --- /dev/null +++ b/test/sandbox/app/views/shared/_action_menu_panel.html.erb @@ -0,0 +1,7 @@ +<%= render ActionMenuComponent.new do |menu| %> + <% menu.with_list do |list| %> + <% list.with_item do %> + <%= t(".menu_action") %> + <% end %> + <% end %> +<% end %> diff --git a/test/sandbox/app/views/shared/_deep_navigation.html.erb b/test/sandbox/app/views/shared/_deep_navigation.html.erb new file mode 100644 index 000000000..690ad6820 --- /dev/null +++ b/test/sandbox/app/views/shared/_deep_navigation.html.erb @@ -0,0 +1,11 @@ +<%= render SectionComponent.new do |section| %> + <% section.with_nav do |nav| %> + <% nav.with_action_menu do |menu| %> + <% menu.with_list do |list| %> + <% list.with_item do %> + <%= t(".deep_action") %> + <% end %> + <% end %> + <% end %> + <% end %> +<% end %> diff --git a/test/sandbox/config/locales/en.yml b/test/sandbox/config/locales/en.yml index 46f26a502..79172933a 100644 --- a/test/sandbox/config/locales/en.yml +++ b/test/sandbox/config/locales/en.yml @@ -28,6 +28,10 @@ en: shared: partial: title: Partial Title + action_menu_panel: + menu_action: Menu Action from Partial + deep_navigation: + deep_action: Deep Action from Partial (5 levels!) rendering_test: i18n_test_component: diff --git a/test/sandbox/test/components/deeply_nested_translation_test.rb b/test/sandbox/test/components/deeply_nested_translation_test.rb new file mode 100644 index 000000000..96e61cd31 --- /dev/null +++ b/test/sandbox/test/components/deeply_nested_translation_test.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +require "test_helper" + +# Tests that translations in deeply nested component blocks (3+ levels) resolve to the +# partial's scope, not an intermediate component's scope. +# +# This pattern is inspired by Primer's ActionMenu → ActionList → Item hierarchy. +class DeeplyNestedTranslationTest < ViewComponent::TestCase + # Tests 3 levels of nesting with typical ViewComponent slot pattern + # + # Structure: + # Partial (_action_menu_panel.html.erb) + # └─> ActionMenuComponent (Level 1) + # └─> menu.with_list → ActionListComponent (Level 2) + # └─> list.with_item → MenuItemComponent (Level 3) + # └─> t(".menu_action") ← Translation should resolve to partial's scope + # + # This uses the standard ViewComponent slot DSL: + # menu.with_list creates an ActionListComponent slot + # list.with_item creates a MenuItemComponent slot + # + # Without the fix, the translation would incorrectly resolve to ActionListComponent's + # scope instead of the partial's scope. + def test_translation_in_3_level_nested_blocks + result = render_inline(ActionMenuPanelWrapperComponent.new) + + assert_includes result.to_html, "Menu Action from Partial" + end + + # Tests 5 levels of nesting to prove the fix works for arbitrary depth + # + # Structure: + # Partial (_deep_navigation.html.erb) + # └─> SectionComponent (Level 1) + # └─> section.with_nav → NavComponent (Level 2) + # └─> nav.with_action_menu → ActionMenuComponent (Level 3) + # └─> menu.with_list → ActionListComponent (Level 4) + # └─> list.with_item → MenuItemComponent (Level 5) + # └─> t(".deep_action") ← Translation should resolve to partial's scope + # + # All using typical ViewComponent slot DSL with renders_one/renders_many. + # + # This demonstrates that the fix isn't just solving "one level deeper" than the original fix, + # but works for any depth of nesting. + def test_translation_in_5_level_nested_blocks + result = render_inline(DeepNavigationWrapperComponent.new) + + assert_includes result.to_html, "Deep Action from Partial (5 levels!)" + end +end