From 2b678bbe9c62f6c583745897edc4b46e90908449 Mon Sep 17 00:00:00 2001
From: jhefferman-sfdc <83669775+jhefferman-sfdc@users.noreply.github.com>
Date: Mon, 13 Jan 2025 14:46:11 -0700
Subject: [PATCH] fix: mismatched scoped / light slots / data (#5131)

* fix: do not render mismatched scoped slotted data

* fix: addressed review comments, added explanations
---
 .../default-slot/modules/x/parent/parent.html |  1 -
 .../src/__tests__/utils/expected-failures.ts  |  4 --
 .../src/compile-js/generate-markup.ts         |  2 +
 .../src/compile-template/index.ts             |  1 +
 .../transformers/component/component.ts       |  3 +-
 .../transformers/component/lwc-component.ts   |  6 +--
 .../transformers/component/slotted-content.ts | 43 +++++++++++++------
 .../src/compile-template/transformers/slot.ts | 32 ++++++++++----
 packages/@lwc/ssr-runtime/src/render.ts       |  8 ++++
 9 files changed, 71 insertions(+), 29 deletions(-)

diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/scoped-slots/default-slot/modules/x/parent/parent.html b/packages/@lwc/engine-server/src/__tests__/fixtures/scoped-slots/default-slot/modules/x/parent/parent.html
index c74cb2caf0..484819fe3e 100644
--- a/packages/@lwc/engine-server/src/__tests__/fixtures/scoped-slots/default-slot/modules/x/parent/parent.html
+++ b/packages/@lwc/engine-server/src/__tests__/fixtures/scoped-slots/default-slot/modules/x/parent/parent.html
@@ -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>
diff --git a/packages/@lwc/ssr-compiler/src/__tests__/utils/expected-failures.ts b/packages/@lwc/ssr-compiler/src/__tests__/utils/expected-failures.ts
index 176201c883..b2f8518054 100644
--- a/packages/@lwc/ssr-compiler/src/__tests__/utils/expected-failures.ts
+++ b/packages/@lwc/ssr-compiler/src/__tests__/utils/expected-failures.ts
@@ -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',
diff --git a/packages/@lwc/ssr-compiler/src/compile-js/generate-markup.ts b/packages/@lwc/ssr-compiler/src/compile-js/generate-markup.ts
index f7be0e745c..ce1fd11d7b 100644
--- a/packages/@lwc/ssr-compiler/src/compile-js/generate-markup.ts
+++ b/packages/@lwc/ssr-compiler/src/compile-js/generate-markup.ts
@@ -25,6 +25,7 @@ const bGenerateMarkup = esTemplate`
             attrs, 
             shadowSlottedContent,
             lightSlottedContent, 
+            scopedSlottedContent,
             parent, 
             scopeToken,
             contextfulParent
@@ -67,6 +68,7 @@ const bGenerateMarkup = esTemplate`
         yield* tmplFn(
             shadowSlottedContent,
             lightSlottedContent,
+            scopedSlottedContent,
             ${/*component class*/ 3},
             instance
         );
diff --git a/packages/@lwc/ssr-compiler/src/compile-template/index.ts b/packages/@lwc/ssr-compiler/src/compile-template/index.ts
index 392ab9c4dc..037d51299f 100644
--- a/packages/@lwc/ssr-compiler/src/compile-template/index.ts
+++ b/packages/@lwc/ssr-compiler/src/compile-template/index.ts
@@ -23,6 +23,7 @@ const bExportTemplate = esTemplate`
     export default async function* tmpl(
             shadowSlottedContent,
             lightSlottedContent,
+            scopedSlottedContent,
             Cmp,
             instance
     ) {
diff --git a/packages/@lwc/ssr-compiler/src/compile-template/transformers/component/component.ts b/packages/@lwc/ssr-compiler/src/compile-template/transformers/component/component.ts
index c0d367fba3..f7f130f119 100644
--- a/packages/@lwc/ssr-compiler/src/compile-template/transformers/component/component.ts
+++ b/packages/@lwc/ssr-compiler/src/compile-template/transformers/component/component.ts
@@ -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
@@ -38,6 +38,7 @@ const bYieldFromChildGenerator = esTemplateWithYield`
             childAttrs, 
             shadowSlottedContent,
             lightSlottedContentMap,
+            scopedSlottedContentMap,
             instance,
             scopeToken,
             contextfulParent
diff --git a/packages/@lwc/ssr-compiler/src/compile-template/transformers/component/lwc-component.ts b/packages/@lwc/ssr-compiler/src/compile-template/transformers/component/lwc-component.ts
index a079c6521a..6653e143d3 100644
--- a/packages/@lwc/ssr-compiler/src/compile-template/transformers/component/lwc-component.ts
+++ b/packages/@lwc/ssr-compiler/src/compile-template/transformers/component/lwc-component.ts
@@ -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
@@ -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),
diff --git a/packages/@lwc/ssr-compiler/src/compile-template/transformers/component/slotted-content.ts b/packages/@lwc/ssr-compiler/src/compile-template/transformers/component/slotted-content.ts
index e785094a1f..6fa1660203 100644
--- a/packages/@lwc/ssr-compiler/src/compile-template/transformers/component/slotted-content.ts
+++ b/packages/@lwc/ssr-compiler/src/compile-template/transformers/component/slotted-content.ts
@@ -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];
             }
         }
 
@@ -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) {
@@ -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[]) => {
@@ -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
     );
diff --git a/packages/@lwc/ssr-compiler/src/compile-template/transformers/slot.ts b/packages/@lwc/ssr-compiler/src/compile-template/transformers/slot.ts
index ac404a0801..3dcdea4adc 100644
--- a/packages/@lwc/ssr-compiler/src/compile-template/transformers/slot.ts
+++ b/packages/@lwc/ssr-compiler/src/compile-template/transformers/slot.ts
@@ -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.
@@ -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 '<!---->';
             }
         }
diff --git a/packages/@lwc/ssr-runtime/src/render.ts b/packages/@lwc/ssr-runtime/src/render.ts
index 97916d3bac..188c169ba7 100644
--- a/packages/@lwc/ssr-runtime/src/render.ts
+++ b/packages/@lwc/ssr-runtime/src/render.ts
@@ -103,6 +103,7 @@ export function renderAttrsNoYield(
 export function* fallbackTmpl(
     _shadowSlottedContent: unknown,
     _lightSlottedContent: unknown,
+    _scopedSlottedContent: unknown,
     Cmp: LightningElementConstructor,
     _instance: unknown
 ) {
@@ -115,6 +116,7 @@ export function fallbackTmplNoYield(
     emit: (segment: string) => void,
     _shadowSlottedContent: unknown,
     _lightSlottedContent: unknown,
+    _scopedSlottedContent: unknown,
     Cmp: LightningElementConstructor,
     _instance: unknown
 ) {
@@ -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,
@@ -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,
@@ -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,
@@ -199,6 +204,7 @@ export async function serverSideRenderComponent(
             null,
             null,
             null,
+            null,
             null
         )) {
             markup += segment;
@@ -213,6 +219,7 @@ export async function serverSideRenderComponent(
             null,
             null,
             null,
+            null,
             null
         );
     } else if (mode === 'sync') {
@@ -225,6 +232,7 @@ export async function serverSideRenderComponent(
             null,
             null,
             null,
+            null,
             null
         );
     } else {