Skip to content

Commit

Permalink
fix: mismatched scoped / light slots / data (#5131)
Browse files Browse the repository at this point in the history
* fix: do not render mismatched scoped slotted data

* fix: addressed review comments, added explanations
  • Loading branch information
jhefferman-sfdc authored Jan 13, 2025
1 parent bedc0a0 commit 2b678bb
Show file tree
Hide file tree
Showing 9 changed files with 71 additions and 29 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
<template>
<x-child>
<!-- TODO [#5020]: Fix rendering of scoped slot content, so that content outside of the template renders correctly with engine-server -->
<span>Slotted content outside of template</span>
<template lwc:slot-data="data">
<span>Slotted content within template {data.id}</span>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,6 @@ export const expectedFailures = new Set([
'exports/component-as-default/index.js',
'known-boolean-attributes/default-def-html-attributes/static-on-component/index.js',
'render-dynamic-value/index.js',
'scoped-slots/advanced/index.js',
'scoped-slots/default-slot/index.js',
'scoped-slots/mixed-with-light-dom-slots-inside/index.js',
'scoped-slots/mixed-with-light-dom-slots-outside/index.js',
'slot-forwarding/slots/mixed/index.js',
'slot-forwarding/slots/dangling/index.js',
'wire/errors/throws-on-computed-key/index.js',
Expand Down
2 changes: 2 additions & 0 deletions packages/@lwc/ssr-compiler/src/compile-js/generate-markup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ const bGenerateMarkup = esTemplate`
attrs,
shadowSlottedContent,
lightSlottedContent,
scopedSlottedContent,
parent,
scopeToken,
contextfulParent
Expand Down Expand Up @@ -67,6 +68,7 @@ const bGenerateMarkup = esTemplate`
yield* tmplFn(
shadowSlottedContent,
lightSlottedContent,
scopedSlottedContent,
${/*component class*/ 3},
instance
);
Expand Down
1 change: 1 addition & 0 deletions packages/@lwc/ssr-compiler/src/compile-template/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ const bExportTemplate = esTemplate`
export default async function* tmpl(
shadowSlottedContent,
lightSlottedContent,
scopedSlottedContent,
Cmp,
instance
) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ const bYieldFromChildGenerator = esTemplateWithYield`
/*
Slotted content is inserted here.
Note that the slotted content will be stored in variables named
`shadowSlottedContent`/`lightSlottedContentMap` which are used below
`shadowSlottedContent`/`lightSlottedContentMap / scopedSlottedContentMap` which are used below
when the child's generateMarkup function is invoked.
*/
is.statement
Expand All @@ -38,6 +38,7 @@ const bYieldFromChildGenerator = esTemplateWithYield`
childAttrs,
shadowSlottedContent,
lightSlottedContentMap,
scopedSlottedContentMap,
instance,
scopeToken,
contextfulParent
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,20 +29,21 @@ const bYieldFromDynamicComponentConstructorGenerator = esTemplateWithYield`
/*
Slotted content is inserted here.
Note that the slotted content will be stored in variables named
`shadowSlottedContent`/`lightSlottedContentMap` which are used below
`shadowSlottedContent`/`lightSlottedContentMap / scopedSlottedContentMap` which are used below
when the child's generateMarkup function is invoked.
*/
is.statement
}
const scopeToken = hasScopedStylesheets ? stylesheetScopeToken : undefined;
yield* Ctor[__SYMBOL__GENERATE_MARKUP](
null,
childProps,
childAttrs,
shadowSlottedContent,
lightSlottedContentMap,
scopedSlottedContentMap,
instance,
scopeToken,
contextfulParent
Expand All @@ -60,7 +61,6 @@ export const LwcComponent: Transformer<IrLwcComponent> = function LwcComponent(n
LightningElement: undefined,
SYMBOL__GENERATE_MARKUP: '__SYMBOL__GENERATE_MARKUP',
});

return bYieldFromDynamicComponentConstructorGenerator(
// The template compiler has validation to prevent lwcIs.value from being a literal
expressionIrToEs(lwcIs.value as IrExpression, cxt),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,18 @@ const bGenerateSlottedContent = esTemplateWithYield`
// Avoid creating the object unnecessarily
: null;
function addLightContent(name, fn) {
let contentList = lightSlottedContentMap[name];
// The containing slot treats scoped slotted content differently.
const scopedSlottedContentMap = ${/* hasScopedSlottedContent */ is.literal}
? Object.create(null)
// Avoid creating the object unnecessarily
: null;
function addSlottedContent(name, fn, contentMap) {
let contentList = contentMap[name];
if (contentList) {
contentList.push(fn);
} else {
lightSlottedContentMap[name] = [fn];
contentMap[name] = [fn];
}
}
Expand All @@ -69,13 +75,13 @@ const bGenerateSlottedContent = esTemplateWithYield`
// Note that this function name (`generateSlottedContent`) does not need to be scoped even though
// it may be repeated multiple times in the same scope, because it's a function _expression_ rather
// than a function _declaration_, so it isn't available to be referenced anywhere.
const bAddLightContent = esTemplate`
addLightContent(${/* slot name */ is.expression} ?? "", async function* generateSlottedContent(contextfulParent, ${
const bAddSlottedContent = esTemplate`
addSlottedContent(${/* slot name */ is.expression} ?? "", async function* generateSlottedContent(contextfulParent, ${
/* scoped slot data variable */ isNullableOf(is.identifier)
}) {
// FIXME: make validation work again
${/* slot content */ false}
});
}, ${/* content map */ is.identifier});
`<EsCallExpression>;

function getShadowSlottedContent(slottableChildren: IrChildNode[], cxt: TransformerContext) {
Expand Down Expand Up @@ -152,7 +158,16 @@ function getLightSlottedContent(rootNodes: IrChildNode[], cxt: TransformerContex
cxt.isSlotted = ancestorIndices.length > 1 || clone.type === 'Slot';
const slotContent = irToEs(clone, cxt);
cxt.isSlotted = originalIsSlotted;
results.push(b.expressionStatement(bAddLightContent(slotName, null, slotContent)));
results.push(
b.expressionStatement(
bAddSlottedContent(
slotName,
null,
slotContent,
b.identifier('lightSlottedContentMap')
)
)
);
};

const traverse = (nodes: IrChildNode[], ancestorIndices: number[]) => {
Expand Down Expand Up @@ -229,23 +244,27 @@ export function getSlottedContent(

// TODO [#4768]: what if the bound variable is `generateMarkup` or some framework-specific identifier?
const addLightContentExpr = b.expressionStatement(
bAddLightContent(slotName, boundVariable, irChildrenToEs(child.children, cxt))
bAddSlottedContent(
slotName,
boundVariable,
irChildrenToEs(child.children, cxt),
b.identifier('scopedSlottedContentMap')
)
);
cxt.popLocalVars();
return addLightContentExpr;
});

const hasShadowSlottedContent = b.literal(shadowSlotContent.length > 0);
const hasLightSlottedContent = b.literal(
lightSlotContent.length > 0 || scopedSlotContent.length > 0
);

const hasLightSlottedContent = b.literal(lightSlotContent.length > 0);
const hasScopedSlottedContent = b.literal(scopedSlotContent.length > 0);
cxt.isSlotted = isSlotted;

return bGenerateSlottedContent(
hasShadowSlottedContent,
shadowSlotContent,
hasLightSlottedContent,
hasScopedSlottedContent,
lightSlotContent,
scopedSlotContent
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 '<!---->';
}
}
} 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) {
// 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.
Expand All @@ -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 '<!---->';
}
}
Expand Down
8 changes: 8 additions & 0 deletions packages/@lwc/ssr-runtime/src/render.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ export function renderAttrsNoYield(
export function* fallbackTmpl(
_shadowSlottedContent: unknown,
_lightSlottedContent: unknown,
_scopedSlottedContent: unknown,
Cmp: LightningElementConstructor,
_instance: unknown
) {
Expand All @@ -115,6 +116,7 @@ export function fallbackTmplNoYield(
emit: (segment: string) => void,
_shadowSlottedContent: unknown,
_lightSlottedContent: unknown,
_scopedSlottedContent: unknown,
Cmp: LightningElementConstructor,
_instance: unknown
) {
Expand All @@ -129,6 +131,7 @@ export type GenerateMarkupFn = (
attrs: Attributes | null,
shadowSlottedContent: AsyncGenerator<string> | null,
lightSlottedContent: Record<number | string, AsyncGenerator<string>> | null,
scopedSlottedContent: Record<number | string, AsyncGenerator<string>> | null,
// Not always null when invoked internally, but should always be
// null when invoked by ssr-runtime
parent: LightningElement | null,
Expand All @@ -143,6 +146,7 @@ export type GenerateMarkupFnAsyncNoGen = (
attrs: Attributes | null,
shadowSlottedContent: AsyncGenerator<string> | null,
lightSlottedContent: Record<number | string, AsyncGenerator<string>> | null,
scopedSlottedContent: Record<number | string, AsyncGenerator<string>> | null,
// Not always null when invoked internally, but should always be
// null when invoked by ssr-runtime
parent: LightningElement | null,
Expand All @@ -157,6 +161,7 @@ export type GenerateMarkupFnSyncNoGen = (
attrs: Attributes | null,
shadowSlottedContent: AsyncGenerator<string> | null,
lightSlottedContent: Record<number | string, AsyncGenerator<string>> | null,
scopedSlottedContent: Record<number | string, AsyncGenerator<string>> | null,
// Not always null when invoked internally, but should always be
// null when invoked by ssr-runtime
parent: LightningElement | null,
Expand Down Expand Up @@ -199,6 +204,7 @@ export async function serverSideRenderComponent(
null,
null,
null,
null,
null
)) {
markup += segment;
Expand All @@ -213,6 +219,7 @@ export async function serverSideRenderComponent(
null,
null,
null,
null,
null
);
} else if (mode === 'sync') {
Expand All @@ -225,6 +232,7 @@ export async function serverSideRenderComponent(
null,
null,
null,
null,
null
);
} else {
Expand Down

0 comments on commit 2b678bb

Please sign in to comment.