fix(ssr): fix adjacent text node concatenation (#5079)

Co-authored-by: jhefferman-sfdc <[email protected]>
Co-authored-by: Will Harney <[email protected]>

Author: 3 people

packages/@lwc/ssr-compiler/src/__tests__/utils/expected-failures.ts

@@ -21,10 +21,6 @@ export const expectedFailures = new Set([
     'scoped-slots/mixed-with-light-dom-slots-outside/index.js',
     'slot-forwarding/slots/mixed/index.js',
     'slot-forwarding/slots/dangling/index.js',
-    'slot-not-at-top-level/with-adjacent-text-nodes/lwcIf-as-sibling/light/index.js',
-    'slot-not-at-top-level/with-adjacent-text-nodes/lwcIf/light/index.js',
-    'slot-not-at-top-level/with-adjacent-text-nodes/if/light/index.js',
-    'slot-not-at-top-level/with-adjacent-text-nodes/if-as-sibling/light/index.js',
     'wire/errors/throws-on-computed-key/index.js',
     'wire/errors/throws-when-colliding-prop-then-method/index.js',
 ]);

packages/@lwc/ssr-compiler/src/compile-template/adjacent-text-nodes.ts

@@ -4,17 +4,33 @@
  * SPDX-License-Identifier: MIT
  * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT
  */
-import { esTemplateWithYield } from '../estemplate';
-import type { IfStatement as EsIfStatement } from 'estree';
+import { builders as b } from 'estree-toolkit/dist/builders';
+import { is } from 'estree-toolkit';
+import { esTemplate, esTemplateWithYield } from '../estemplate';
+import { isLiteral } from './shared';
+import { expressionIrToEs } from './expression';
+import type {
+    CallExpression as EsCallExpression,
+    Expression as EsExpression,
+    ExpressionStatement as EsExpressionStatement,
+} from 'estree';
 import type { TransformerContext } from './types';
-import type { Node as IrNode } from '@lwc/template-compiler';
+import type { Node as IrNode, Text as IrText, Comment as IrComment } from '@lwc/template-compiler';
+
+const bNormalizeTextContent = esTemplate`
+    normalizeTextContent(${/* string value */ is.expression});
+`<EsCallExpression>;
+
+const bYieldTextContent = esTemplateWithYield`
+    yield renderTextContent(${/* text concatenation, possibly as binary expression */ is.expression});
+`<EsExpressionStatement>;
 
 /**
  * True if this is one of a series of text content nodes and/or comment node that are adjacent to one another as
  * siblings. (Comment nodes are ignored when preserve-comments is turned off.) This allows for adjacent text
  * node concatenation.
  */
-const isConcatenatedNode = (node: IrNode, cxt: TransformerContext) => {
+const isConcatenatedNode = (node: IrNode, cxt: TransformerContext): node is IrText | IrComment => {
     switch (node.type) {
         case 'Text':
             return true;
@@ -26,23 +42,62 @@ const isConcatenatedNode = (node: IrNode, cxt: TransformerContext) => {
 };
 
 export const isLastConcatenatedNode = (cxt: TransformerContext) => {
-    const { nextSibling } = cxt;
+    const siblings = cxt.siblings!;
+    const currentNodeIndex = cxt.currentNodeIndex!;
+
+    const nextSibling = siblings[currentNodeIndex + 1];
     if (!nextSibling) {
         // we are the last sibling
         return true;
     }
     return !isConcatenatedNode(nextSibling, cxt);
 };
 
-export const bYieldTextContent = esTemplateWithYield`
-    if (didBufferTextContent) {
-        // We are at the end of a series of text nodes - flush to a concatenated string
-        // We only render the ZWJ if there were actually any dynamic text nodes rendered
-        // The ZWJ is just so hydration can compare the SSR'd dynamic text content against
-        // the CSR'd text content.
-        yield textContentBuffer === '' ? '\u200D' : htmlEscape(textContentBuffer);
-        // Reset
-        textContentBuffer = '';
-        didBufferTextContent = false;
+function generateExpressionFromTextNode(node: IrText, cxt: TransformerContext) {
+    return isLiteral(node.value) ? b.literal(node.value.value) : expressionIrToEs(node.value, cxt);
+}
+
+export function generateConcatenatedTextNodesExpressions(cxt: TransformerContext) {
+    const siblings = cxt.siblings!;
+    const currentNodeIndex = cxt.currentNodeIndex!;
+
+    const textNodes = [];
+
+    for (let i = currentNodeIndex; i >= 0; i--) {
+        const sibling = siblings[i];
+        if (isConcatenatedNode(sibling, cxt)) {
+            if (sibling.type === 'Text') {
+                textNodes.unshift(sibling);
+            }
+        } else {
+            // If we reach a non-Text/Comment node, we are done. These should not be concatenated
+            // with sibling Text nodes separated by e.g. an Element:
+            //     {a}{b}<div></div>{c}{d}
+            // In the above, {a} and {b} are concatenated, and {c} and {d} are concatenated,
+            // but the `<div>` separates the two groups.
+            break;
+        }
     }
-`<EsIfStatement>;
+
+    if (!textNodes.length) {
+        // Render nothing. This can occur if we hit a comment in non-preserveComments mode with no adjacent text nodes
+        return [];
+    }
+
+    cxt.import(['normalizeTextContent', 'renderTextContent']);
+
+    // Generate a binary expression to concatenate the text together. E.g.:
+    //     renderTextContent(
+    //         normalizeTextContent(a) +
+    //         normalizeTextContent(b) +
+    //         normalizeTextContent(c)
+    //     )
+    const concatenatedExpression = textNodes
+        .map(
+            (node) =>
+                bNormalizeTextContent(generateExpressionFromTextNode(node, cxt)) as EsExpression
+        )
+        .reduce((accumulator, expression) => b.binaryExpression('+', accumulator, expression));
+
+    return [bYieldTextContent(concatenatedExpression)];
+}

packages/@lwc/ssr-compiler/src/compile-template/context.ts

@@ -42,6 +42,8 @@ export function createNewContext(templateOptions: TemplateOpts): {
             isLocalVar,
             templateOptions,
             import: importManager.add.bind(importManager),
+            siblings: undefined,
+            currentNodeIndex: undefined,
         },
     };
 }

packages/@lwc/ssr-compiler/src/compile-template/ir-to-es.ts

@@ -75,15 +75,16 @@ export function irChildrenToEs(
     const result: EsStatement[] = [];
 
     for (let i = 0; i < children.length; i++) {
-        cxt.prevSibling = children[i - 1];
-        cxt.nextSibling = children[i + 1];
+        // must set the siblings inside the for loop due to nested children
+        cxt.siblings = children;
+        cxt.currentNodeIndex = i;
         const cleanUp = cb?.(children[i]);
         result.push(...irToEs(children[i], cxt));
         cleanUp?.();
     }
-
-    cxt.prevSibling = undefined;
-    cxt.nextSibling = undefined;
+    // reset the context
+    cxt.siblings = undefined;
+    cxt.currentNodeIndex = undefined;
 
     return result;
 }

packages/@lwc/ssr-compiler/src/compile-template/transformers/comment.ts

@@ -7,21 +7,25 @@
 
 import { builders as b } from 'estree-toolkit';
 
-import { bYieldTextContent, isLastConcatenatedNode } from '../adjacent-text-nodes';
+import {
+    generateConcatenatedTextNodesExpressions,
+    isLastConcatenatedNode,
+} from '../adjacent-text-nodes';
 import type { Comment as IrComment } from '@lwc/template-compiler';
 import type { Transformer } from '../types';
 
 export const Comment: Transformer<IrComment> = function Comment(node, cxt) {
     if (cxt.templateOptions.preserveComments) {
         return [b.expressionStatement(b.yieldExpression(b.literal(`<!--${node.value}-->`)))];
     } else {
-        cxt.import('htmlEscape');
-
         const isLastInSeries = isLastConcatenatedNode(cxt);
 
         // If preserve comments is off, we check if we should flush text content
         // for adjacent text nodes. (If preserve comments is on, then the previous
         // text node already flushed.)
-        return [...(isLastInSeries ? [bYieldTextContent()] : [])];
+        if (isLastInSeries) {
+            return generateConcatenatedTextNodesExpressions(cxt);
+        }
+        return [];
     }
 };

packages/@lwc/ssr-compiler/src/compile-template/transformers/component/slotted-content.ts

@@ -13,6 +13,7 @@ import { irChildrenToEs, irToEs } from '../../ir-to-es';
 import { isLiteral } from '../../shared';
 import { expressionIrToEs } from '../../expression';
 import { isNullableOf } from '../../../estree/validators';
+import { isLastConcatenatedNode } from '../../adjacent-text-nodes';
 import type { CallExpression as EsCallExpression, Expression as EsExpression } from 'estree';
 
 import type {
@@ -156,6 +157,9 @@ function getLightSlottedContent(rootNodes: IrChildNode[], cxt: TransformerContex
 
     const traverse = (nodes: IrChildNode[], ancestorIndices: number[]) => {
         for (let i = 0; i < nodes.length; i++) {
+            // must set the siblings inside the for loop due to nested children
+            cxt.siblings = nodes;
+            cxt.currentNodeIndex = i;
             const node = nodes[i];
             switch (node.type) {
                 // SlottableAncestorIrType
@@ -175,11 +179,21 @@ function getLightSlottedContent(rootNodes: IrChildNode[], cxt: TransformerContex
                     // '' is the default slot name. Text nodes are always slotted into the default slot
                     const slotName =
                         node.type === 'Text' ? b.literal('') : bAttributeValue(node, 'slot');
+
+                    // For concatenated adjacent text nodes, for any but the final text node, we
+                    // should skip them and let the final text node take care of rendering its siblings
+                    if (node.type === 'Text' && !isLastConcatenatedNode(cxt)) {
+                        continue;
+                    }
+
                     addLightDomSlotContent(slotName, [...ancestorIndices, i]);
                     break;
                 }
             }
         }
+        // reset the context
+        cxt.siblings = undefined;
+        cxt.currentNodeIndex = undefined;
     };
 
     traverse(rootNodes, []);

packages/@lwc/ssr-compiler/src/compile-template/transformers/text.ts

@@ -5,32 +5,20 @@
  * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT
  */
 
-import { builders as b, is } from 'estree-toolkit';
-import { esTemplateWithYield } from '../../estemplate';
-import { expressionIrToEs } from '../expression';
-import { isLiteral } from '../shared';
-
-import { bYieldTextContent, isLastConcatenatedNode } from '../adjacent-text-nodes';
-import type {
-    Statement as EsStatement,
-    ExpressionStatement as EsExpressionStatement,
-} from 'estree';
+import {
+    generateConcatenatedTextNodesExpressions,
+    isLastConcatenatedNode,
+} from '../adjacent-text-nodes';
+import type { Statement as EsStatement } from 'estree';
 import type { Text as IrText } from '@lwc/template-compiler';
 import type { Transformer } from '../types';
 
-const bBufferTextContent = esTemplateWithYield`
-    didBufferTextContent = true;
-    textContentBuffer += massageTextContent(${/* string value */ is.expression});
-`<EsExpressionStatement[]>;
-
 export const Text: Transformer<IrText> = function Text(node, cxt): EsStatement[] {
-    cxt.import(['htmlEscape', 'massageTextContent']);
-
-    const isLastInSeries = isLastConcatenatedNode(cxt);
-
-    const valueToYield = isLiteral(node.value)
-        ? b.literal(node.value.value)
-        : expressionIrToEs(node.value, cxt);
+    if (isLastConcatenatedNode(cxt)) {
+        // render all concatenated content up to us
+        return generateConcatenatedTextNodesExpressions(cxt);
+    }
 
-    return [...bBufferTextContent(valueToYield), ...(isLastInSeries ? [bYieldTextContent()] : [])];
+    // our last sibling is responsible for rendering our content, not us
+    return [];
 };

packages/@lwc/ssr-compiler/src/compile-template/types.ts

@@ -18,8 +18,8 @@ export interface TransformerContext {
     popLocalVars: () => void;
     isLocalVar: (varName: string | null | undefined) => boolean;
     templateOptions: TemplateOpts;
-    prevSibling?: IrNode;
-    nextSibling?: IrNode;
+    siblings: IrNode[] | undefined;
+    currentNodeIndex: number | undefined;
     isSlotted?: boolean;
     import: (
         imports: string | string[] | Record<string, string | undefined>,

packages/@lwc/ssr-runtime/src/index.ts

@@ -30,7 +30,7 @@ export {
     // renderComponent is an alias for serverSideRenderComponent
     serverSideRenderComponent as renderComponent,
 } from './render';
-export { massageTextContent } from './render-text-content';
+export { normalizeTextContent, renderTextContent } from './render-text-content';
 export { hasScopedStaticStylesheets, renderStylesheets } from './styles';
 export { toIteratorDirective } from './to-iterator-directive';
 export { validateStyleTextContents } from './validate-style-text-contents';

packages/@lwc/ssr-runtime/src/render-text-content.ts

@@ -5,12 +5,28 @@
  * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT
  */
 
+import { htmlEscape } from '@lwc/shared';
+
 /**
- * Given an object, render it for use as a text content node.
+ * Given an object, render it for use as a text content node. Not that this applies to individual text nodes,
+ * not the concatenated result of multiple adjacent text nodes.
  * @param value
  */
-export function massageTextContent(value: unknown): string {
+export function normalizeTextContent(value: unknown): string {
     // Using non strict equality to align with original implementation (ex. undefined == null)
     // See: https://github.com/salesforce/lwc/blob/348130f/packages/%40lwc/engine-core/src/framework/api.ts#L548
     return value == null ? '' : String(value);
 }
+
+/**
+ * Given a string, render it for use as text content in HTML. Notably this escapes HTML and renders as
+ * a ZWJ is empty. Intended to be used on the result of concatenating multiple adjacent text nodes together.
+ * @param value
+ */
+export function renderTextContent(value: string): string {
+    // We are at the end of a series of text nodes - flush to a concatenated string
+    // We only render the ZWJ if there were actually any dynamic text nodes rendered
+    // The ZWJ is just so hydration can compare the SSR'd dynamic text content against
+    // the CSR'd text content.
+    return value === '' ? '\u200D' : htmlEscape(value);
+}