Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@

## 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.

Check warning on line 13 in docs/CHANGELOG.md

View workflow job for this annotation

GitHub Actions / prose

[vale] reported by reviewdog 🐶 [Microsoft.Adverbs] Consider removing 'deeply'. Raw Output: {"message": "[Microsoft.Adverbs] Consider removing 'deeply'.", "location": {"path": "docs/CHANGELOG.md", "range": {"start": {"line": 13, "column": 110}}}, "severity": "WARNING"}

Check warning on line 13 in docs/CHANGELOG.md

View workflow job for this annotation

GitHub Actions / prose

[vale] reported by reviewdog 🐶 [Microsoft.Adverbs] Consider removing 'deeply'. Raw Output: {"message": "[Microsoft.Adverbs] Consider removing 'deeply'.", "location": {"path": "docs/CHANGELOG.md", "range": {"start": {"line": 13, "column": 39}}}, "severity": "WARNING"}

*Nathaniel Watts*

* Allow `render_inline` with Nokogiri::HTML5 to parse more arbitrary content including bare table content otherwise illegal fragments like `<td>`.

*Jonathan Rochkind*
Expand Down
11 changes: 7 additions & 4 deletions lib/view_component/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand All @@ -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
Expand Down
6 changes: 3 additions & 3 deletions lib/view_component/slot.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down
11 changes: 9 additions & 2 deletions lib/view_component/slotable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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)
Expand Down
15 changes: 15 additions & 0 deletions test/sandbox/app/components/action_list_component.rb
Original file line number Diff line number Diff line change
@@ -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
13 changes: 13 additions & 0 deletions test/sandbox/app/components/action_menu_component.rb
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# frozen_string_literal: true

class ActionMenuPanelWrapperComponent < ViewComponent::Base
def call
render "shared/action_menu_panel"
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# frozen_string_literal: true

class DeepNavigationWrapperComponent < ViewComponent::Base
def call
render "shared/deep_navigation"
end
end
11 changes: 11 additions & 0 deletions test/sandbox/app/components/menu_item_component.rb
Original file line number Diff line number Diff line change
@@ -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
12 changes: 12 additions & 0 deletions test/sandbox/app/components/nav_component.rb
Original file line number Diff line number Diff line change
@@ -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
12 changes: 12 additions & 0 deletions test/sandbox/app/components/section_component.rb
Original file line number Diff line number Diff line change
@@ -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
<section class="section">
<%= nav %>
</section>
ERB
end
7 changes: 7 additions & 0 deletions test/sandbox/app/views/shared/_action_menu_panel.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<%= render ActionMenuComponent.new do |menu| %>
<% menu.with_list do |list| %>
<% list.with_item do %>
<%= t(".menu_action") %>
<% end %>
<% end %>
<% end %>
11 changes: 11 additions & 0 deletions test/sandbox/app/views/shared/_deep_navigation.html.erb
Original file line number Diff line number Diff line change
@@ -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 %>
4 changes: 4 additions & 0 deletions test/sandbox/config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
51 changes: 51 additions & 0 deletions test/sandbox/test/components/deeply_nested_translation_test.rb
Original file line number Diff line number Diff line change
@@ -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
Loading