-
Notifications
You must be signed in to change notification settings - Fork 439
fix: mismatched scoped / light slots / data #5131
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,22 +25,38 @@ const bConditionalSlot = esTemplateWithYield` | |
| if (isLightDom) { | ||
| const isScopedSlot = ${/* isScopedSlot */ is.literal}; | ||
| const isSlotted = ${/* isSlotted */ is.literal}; | ||
| const slotName = ${/* slotName */ is.expression}; | ||
| const lightGenerators = lightSlottedContent?.[slotName ?? ""]; | ||
| const scopedGenerators = scopedSlottedContent?.[slotName ?? ""]; | ||
| const mismatchedSlots = isScopedSlot ? lightGenerators : scopedGenerators; | ||
| const generators = isScopedSlot ? scopedGenerators : lightGenerators; | ||
|
|
||
| // start bookend HTML comment for light DOM slot vfragment | ||
| if (!isSlotted) { | ||
| yield '<!---->'; | ||
|
|
||
| // scoped slot factory has its own vfragment hence its own bookend | ||
| if (isScopedSlot) { | ||
| // If there is slot data, scoped slot factory has its own vfragment hence its own bookend | ||
| if (isScopedSlot && generators) { | ||
| yield '<!---->'; | ||
| } | ||
| } | ||
|
|
||
| const generators = lightSlottedContent?.[${/* slotName */ is.expression} ?? ""]; | ||
| if (generators) { | ||
| for (const generator of generators) { | ||
| yield* generator(contextfulParent, ${/* scoped slot data */ isNullableOf(is.expression)}); | ||
| for (let i = 0; i < generators.length; i++) { | ||
| yield* generators[i](contextfulParent, ${/* scoped slot data */ isNullableOf(is.expression)}); | ||
| // Scoped slotted data is separated by bookends. Final bookends are added outside of the loop below. | ||
| if (isScopedSlot && i < generators.length - 1) { | ||
| yield '<!---->'; | ||
| yield '<!---->'; | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is really bizarre behavior! I wonder what explains it.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh wait I think I see – it's because of the double bookend? So the double bookend logic for the start/end is handled elsewhere? It might be good to add a comment explaining this!
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep exactly. Added a note to explain that! |
||
| } | ||
| } else { | ||
| /* | ||
| If there were mismatched slots, do not fallback to the default. This is required for parity with | ||
| engine-core which resets children to an empty array when there are children (mismatched or not). | ||
| Because the child nodes are reset, the default slotted content is not rendered in the mismatched slot case. | ||
| See https://github.com/salesforce/lwc/blob/master/packages/%40lwc/engine-core/src/framework/api.ts#L238 | ||
| */ | ||
| } else if (!mismatchedSlots) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd love to understand why mismatched slots have any impact here.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added a comment here to explain: This is required for parity with engine-core which resets children to an empty array when there are children (mismatched or not). Because the child nodes are reset, the default slotted content is not rendered in the mismatched slot case. See here. I'm guessing this is a sort-of-bug but fixing it would mean a v1/client behavior change and I don't think it's worth it.... |
||
| // If we're in this else block, then the generator _must_ have yielded | ||
| // something. It's impossible for a slottedContent["foo"] to exist | ||
| // without the generator yielding at least a text node / element. | ||
|
|
@@ -53,8 +69,8 @@ const bConditionalSlot = esTemplateWithYield` | |
| if (!isSlotted) { | ||
| yield '<!---->'; | ||
|
|
||
| // scoped slot factory has its own vfragment hence its own bookend | ||
| if (isScopedSlot) { | ||
| // If there is slot data, scoped slot factory has its own vfragment hence its own bookend | ||
| if (isScopedSlot && generators) { | ||
| yield '<!---->'; | ||
| } | ||
| } | ||
|
|
||
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.