Merge pull request #2520 from thewatts/nw/nested-partial-translations

Update storing of virtual path for deep nesting

Author: joelhawksley

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.
+
+    *Nathaniel Watts*
+
 * Allow `render_inline` with Nokogiri::HTML5 to parse more arbitrary content including bare table content otherwise illegal fragments like `<td>`.
 
     *Jonathan Rochkind*

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

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)

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)

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
+    <ul class="action-list">
+      <% items.each do |item| %>
+        <%= item %>
+      <% end %>
+    </ul>
+  ERB
+end

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
+    <div class="action-menu">
+      <%= list %>
+    </div>
+  ERB
+end

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

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

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
+    <li class="menu-item">
+      <%= content %>
+    </li>
+  ERB
+end

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
+    <nav class="nav">
+      <%= action_menu %>
+    </nav>
+  ERB
+end