Update storing of virtual path for deep nesting #2520
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What are you trying to accomplish?
Fix translation scope resolution in deeply nested component blocks (3+ levels). Currently, translations called inside deeply nested slot blocks using
renders_many/renders_oneincorrectly resolve to an intermediate component's scope instead of the partial's scope where the block was defined.Example of the bug:
The translation incorrectly resolves to
action_list_component.menu_actioninstead ofshared.action_menu_panel.menu_action.Builds on #2389.
Fixes #2386.
What approach did you choose and why?
The previous fix in #2389 added
with_original_virtual_path, which restored the virtual path one level up to the immediate parent component.This worked for 2-level nesting:
But failed for 3+ level nesting:
The issue: The method only went one level up to the immediate parent, not all the way back to the original partial.
Approach: Capture the virtual path at block definition time instead of trying to walk back up the component hierarchy at execution time.
Implementation details:
with_item), capture the current@virtual_pathand store it on the slot as__vc_content_block_virtual_path@parent.with_captured_virtual_path(@__vc_content_block_virtual_path)to temporarily restore the captured pathwith_captured_virtual_pathmethod takes the captured path as an explicit parameter and temporarily sets the virtual path during block executionWhy this approach:
Alternative approaches considered:
Tradeoffs:
__vc_content_block_virtual_path) to each slot that has a blockAfter this fix, deeply nested translations work correctly:
Anything you want to highlight for special attention from reviewers?
Test coverage: Added comprehensive tests for both 3-level and 5-level nesting scenarios using simplified test components inspired by Primer's ActionMenu pattern. The tests verify that translations resolve to the partial's scope, not any intermediate component's scope.
Implementation note: This also consolidates the virtual path restoration logic by having all three execution contexts (main component content blocks, lambda slots, and component class slots) use the same
with_captured_virtual_pathmethod with an explicit parameter, making the behavior consistent everywhere.