Fix i18n scope for nested lambda-backed slots#2633
Open
artinboghosian wants to merge 2 commits into
Open
Conversation
Author
|
@joelhawksley The failing check is also failing on main branch. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 relative t(".key") translations inside nested lambda-backed slots resolving against the wrong scope. Instead of resolving to the partial where the block was defined, they resolved to an intermediate component's virtual path.
Fixes the bug reported in #2513 which persisted after #2520.
What approach did you choose and why?
PR #2520 introduced
__vc_content_block_virtual_path— capturing the virtual path at block-definition time and restoring it at block-evaluation time. This correctly fixed non-lambda slots via Slot#to_s.However, lambda-backed slots are evaluated immediately inside
__vc_set_slotrather than deferred toSlot#to_s. The lambda branch was updated in #2520 to wrap the block evaluation withwith_captured_virtual_path, but used@old_virtual_path(the intermediate parent component's path) instead of the already-capturedslot.__vc_content_block_virtual_path(the original partial's path).The existing
slot.__vc_content_block_virtual_pathis defined viaattr_writer, making it write-only. Rather than exposing it through anattr_reader, the captured path is stored in a local variable within __vc_set_slot and passed directly to the lambda branch. Happy to use a different approach if the project prefers exposing the reader instead.