-
Notifications
You must be signed in to change notification settings - Fork 427
fix: deduplicate local variables for SSRv2 #5554
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| <template lwc:render-mode="light"> | ||
| <template for:each={relatedListData} for:item="list" if:true={relatedListData}> | ||
| <template for:each={list.recordIds} for:item="recordId"> | ||
| <template for:each={list.fields} for:item="field" for:index="index"> | ||
| <x-child key={field}> | ||
| <div>{field}</div> | ||
| </x-child> | ||
| </template> | ||
| </template> | ||
| </template> | ||
| </template> No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before the change in this PR the output is this:
const __lwcGenerateShadowSlottedContent_xChild_0 = (list, __unused__, recordId, __unused__, field, index) => function __lwcGenerateShadowSlottedContent_xChild_0The double __unused__ led to a compilation error.
After this change it now appears as:
const __lwcGenerateShadowSlottedContent_xChild_0 = (list, __unused__, recordId, field, index) => function __lwcGenerateShadowSlottedContent_xChild_0There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's less likely to occur with user-provided variables, but deduping is also correct in that case because re-declared variables are shadowed; there's only one "version" of the variable in the scope where we call this function.
| const getLocalVars = () => localVarStack.flatMap((varsSet) => Array.from(varsSet)); | ||
| // Unique local variable names across block scopes, we don't care about block scope order | ||
| const getLocalVars = () => [ | ||
| ...new Set(localVarStack.flatMap((varsSet) => Array.from(varsSet))), | ||
| ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the main change, essentially we're just deduplicating the list.
Note that based on how we're using the array we don't really care about the order the variables appear in the block scopes.
| @@ -0,0 +1,2 @@ | |||
| <fixture-test> | |||
| </fixture-test> No newline at end of file | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this supposed to be empty? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like I messed up the data in the array oops, I'll fix it.
wjhsf
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nolanlawson would approve of the ratio of test files to lines of production code changed.
...-each-block/nested-no-index/modules/x/for-each-nested-no-index/for-each-nested-no-index.html
Outdated
Show resolved
Hide resolved
| <template lwc:render-mode="light"> | ||
| <template for:each={relatedListData} for:item="list" if:true={relatedListData}> | ||
| <template for:each={list.recordIds} for:item="recordId"> | ||
| <template for:each={list.fields} for:item="field" for:index="index"> | ||
| <x-child key={field}> | ||
| <div>{field}</div> | ||
| </x-child> | ||
| </template> | ||
| </template> | ||
| </template> | ||
| </template> No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's less likely to occur with user-provided variables, but deduping is also correct in that case because re-declared variables are shadowed; there's only one "version" of the variable in the scope where we call this function.
Details
The SSR v2 compiler keeps track of local variables via a stack.
When the local variables are retrieved we don't deduplicate them which can lead to errors, specifically:
LWC1001: Unexpected compilation error: Duplicate argument name "unused":
This PR introduces a fix by deduping the variables before use.
Does this pull request introduce a breaking change?
Does this pull request introduce an observable change?
GUS work item
W-19971203