Skip to content
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

fix(ssr): fix adjacent text node concatenation within slotted content #5069

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<x-outer>
<div>
Outer!
</div>
<x-inner>
<div>
Inner!
</div>
<!---->
‍‍
<x-deep>
<div>
Deep!
</div>
<!---->
<div>
</div>
<!---->
</x-deep>
‍‍
<!---->
</x-inner>
</x-outer>
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<x-outer>
<div>
Outer!
</div>
<x-inner>
<div>
Inner!
</div>
<!---->
‍‍‍‍
<x-deep>
<div>
Deep!
</div>
<!---->
‍‍
<div>
</div>
‍‍
<!---->
</x-deep>
‍‍‍‍
<!---->
</x-inner>
</x-outer>
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ export const expectedFailures = new Set([
'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',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,33 +6,6 @@
*/
import { esTemplateWithYield } from '../estemplate';
import type { IfStatement as EsIfStatement } from 'estree';
import type { TransformerContext } from './types';
import type { Node as IrNode } from '@lwc/template-compiler';

/**
* 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) => {
switch (node.type) {
case 'Text':
return true;
case 'Comment':
return !cxt.templateOptions.preserveComments;
default:
return false;
}
};

export const isLastConcatenatedNode = (cxt: TransformerContext) => {
const { nextSibling } = cxt;
if (!nextSibling) {
// we are the last sibling
return true;
}
return !isConcatenatedNode(nextSibling, cxt);
};

export const bYieldTextContent = esTemplateWithYield`
if (didBufferTextContent) {
Expand Down
93 changes: 81 additions & 12 deletions packages/@lwc/ssr-compiler/src/compile-template/ir-to-es.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,23 +8,33 @@
import { inspect } from 'util';

import { is, builders as b } from 'estree-toolkit';
import { esTemplate } from '../estemplate';
import { esTemplate, esTemplateWithYield } from '../estemplate';
import { Comment } from './transformers/comment';
import { Component, LwcComponent } from './transformers/component';
import { Element } from './transformers/element';
import { ForEach } from './transformers/for-each';
import { ForOf } from './transformers/for-of';
import { LegacyIf } from './transformers/legacyIf';
import { Slot } from './transformers/slot';
import { Text } from './transformers/text';
import { createNewContext } from './context';
import { IfBlock } from './transformers/lwcIf';
import { isLiteral } from './shared';
import { expressionIrToEs } from './expression';
import { Text } from './transformers/text';
import type {
ChildNode as IrChildNode,
Comment as IrComment,
Node as IrNode,
Root as IrRoot,
Text as IrText,
} from '@lwc/template-compiler';
import type { Statement as EsStatement, ThrowStatement as EsThrowStatement } from 'estree';
import type {
Statement as EsStatement,
ThrowStatement as EsThrowStatement,
CallExpression as EsCallExpression,
BlockStatement as EsBlockStatement,
BinaryExpression as EsBinaryExpression,
} from 'estree';
import type { TemplateOpts, Transformer, TransformerContext } from './types';

const bThrowError = esTemplate`
Expand Down Expand Up @@ -67,23 +77,82 @@ const transformers: Transformers = {
Lwc: LwcComponent,
};

const irChildToEs = (
child: IrChildNode,
cxt: TransformerContext,
cb?: (child: IrChildNode) => void | (() => void)
) => {
const cleanUp = cb?.(child);
const result = irToEs(child, cxt);
cleanUp?.();
return result;
};

const isConcatenatableTextNode = (
child: IrChildNode,
cxt: TransformerContext
): child is IrComment | IrText =>
child.type === 'Text' || (child.type === 'Comment' && !cxt.templateOptions.preserveComments);

const bMassageTextContent = esTemplate`
massageTextContent(${/* string value */ is.expression});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought we renamed this to normalizeTextContent. I guess not. 🤔

`<EsCallExpression>;

const bYieldTextContent = esTemplateWithYield`
// 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.
{
const text = ${/* string value */ is.expression};
yield text === '' ? '\u200D' : htmlEscape(text);
}
`<EsBlockStatement>;

const mergeTextNodes = (nodes: Array<IrText>, cxt: TransformerContext): EsStatement => {
cxt.import(['htmlEscape', 'massageTextContent']);

const mergedExpression = nodes
.map((node) =>
isLiteral(node.value) ? b.literal(node.value.value) : expressionIrToEs(node.value, cxt)
)
.map((expression) => bMassageTextContent(expression))
.reduce(
// @ts-expect-error FIXME later
(accumulator: EsBinaryExpression, expression: EsCallExpression | EsBinaryExpression) =>
b.binaryExpression('+', accumulator, expression)
);

return bYieldTextContent(mergedExpression);
};

export function irChildrenToEs(
children: IrChildNode[],
cxt: TransformerContext,
cb?: (child: IrChildNode) => (() => void) | void
): EsStatement[] {
const result: EsStatement[] = [];

for (let i = 0; i < children.length; i++) {
cxt.prevSibling = children[i - 1];
cxt.nextSibling = children[i + 1];
const cleanUp = cb?.(children[i]);
result.push(...irToEs(children[i], cxt));
cleanUp?.();
const adjacentTextNodes: Array<IrText> = [];
for (const child of children) {
if (isConcatenatableTextNode(child, cxt)) {
// Don't add this node yet -- wait until we've found all adjacent text nodes
if (child.type === 'Text') {
adjacentTextNodes.push(child);
}
} else {
if (adjacentTextNodes.length > 0) {
// We've reached another node type -- flush the buffer of text nodes
result.push(mergeTextNodes(adjacentTextNodes, cxt));
adjacentTextNodes.length = 0;
}
result.push(...irChildToEs(child, cxt, cb));
}
}

cxt.prevSibling = undefined;
cxt.nextSibling = undefined;
// Add any remaining text nodes
if (adjacentTextNodes.length > 0) {
result.push(mergeTextNodes(adjacentTextNodes, cxt));
}

return result;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,22 +6,13 @@
*/

import { builders as b } from 'estree-toolkit';

import { bYieldTextContent, 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()] : [])];
return [];
}
};
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,15 @@ 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');

const metadata = getTextNodeMetadata(node, parent)
if (metadata.isSkippable) {
continue
}
if (metadata.hasGlommedNodes) {

}

addLightDomSlotContent(slotName, [...ancestorIndices, i]);
break;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,27 +10,22 @@ 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 type { Statement as EsStatement, BlockStatement as EsBlockStatement } 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[]>;
const bYieldTextContent = esTemplateWithYield`
{
const text = massageTextContent(${/* string value */ is.expression});
yield text === '' ? '\u200D' : htmlEscape(text);
}`<EsBlockStatement>;

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);

return [...bBufferTextContent(valueToYield), ...(isLastInSeries ? [bYieldTextContent()] : [])];
return [bYieldTextContent(valueToYield)];
};
2 changes: 0 additions & 2 deletions packages/@lwc/ssr-compiler/src/compile-template/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ export interface TransformerContext {
popLocalVars: () => void;
isLocalVar: (varName: string | null | undefined) => boolean;
templateOptions: TemplateOpts;
prevSibling?: IrNode;
nextSibling?: IrNode;
isSlotted?: boolean;
import: (
imports: string | string[] | Record<string, string | undefined>,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/*
* Copyright (c) 2024, Salesforce, Inc.
* All rights reserved.
* SPDX-License-Identifier: MIT
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT
*/
import type { BaseParentNode, Text } from './types';

export function getPreviousAdjacentSiblings(node: Text, parent: BaseParentNode, preserveComments: boolean) {
const siblings = []
for (let i = 0; i < parent.children.length; i++) {
const otherChild = parent.children[i]
if (otherChild === node) {
return siblings
} else if (node.type === 'Text' || (node.type === 'Comment' && !preserveComments)) {
siblings.push(node)
}
}
}
Loading