From cdbf06c6840f4757b84d4969570d421a8e73ce1c Mon Sep 17 00:00:00 2001 From: Nolan Lawson Date: Fri, 15 Nov 2024 13:04:35 -0800 Subject: [PATCH] fix(ssr): make sync mode default, fix it (#4869) --- packages/@lwc/compiler/src/options.ts | 4 +- .../rollup.config.mjs | 3 - ...ight-slot-create-container-5k.benchmark.js | 3 +- .../slot-create-container-5k.benchmark.js | 3 +- .../ssr/table/table-render-10k.benchmark.js | 13 +---- .../table/tablecmp-render-10k.benchmark.js | 13 +---- .../src/utils/styledComponentSsrBenchmark.js | 5 +- packages/@lwc/shared/src/index.ts | 1 + packages/@lwc/shared/src/ssr.ts | 7 +++ .../src/__tests__/fixtures.spec.ts | 3 +- .../src/__tests__/utils/expected-failures.ts | 1 - .../transformers/component.ts | 7 ++- packages/@lwc/ssr-compiler/src/index.ts | 6 +- .../@lwc/ssr-compiler/src/transmogrify.ts | 58 ++++++++++++------- packages/@lwc/ssr-runtime/src/render.ts | 4 +- 15 files changed, 69 insertions(+), 62 deletions(-) create mode 100644 packages/@lwc/shared/src/ssr.ts diff --git a/packages/@lwc/compiler/src/options.ts b/packages/@lwc/compiler/src/options.ts index c1d4ca203b..31c44adfc7 100755 --- a/packages/@lwc/compiler/src/options.ts +++ b/packages/@lwc/compiler/src/options.ts @@ -5,7 +5,7 @@ * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT */ import { InstrumentationObject, CompilerValidationErrors, invariant } from '@lwc/errors'; -import { isUndefined, isBoolean, getAPIVersionFromNumber } from '@lwc/shared'; +import { isUndefined, isBoolean, getAPIVersionFromNumber, DEFAULT_SSR_MODE } from '@lwc/shared'; import { CompilationMode } from '@lwc/ssr-compiler'; import type { CustomRendererConfig } from '@lwc/template-compiler'; @@ -33,7 +33,7 @@ const DEFAULT_OPTIONS = { disableSyntheticShadowSupport: false, enableLightningWebSecurityTransforms: false, targetSSR: false, - ssrMode: 'asyncYield', + ssrMode: DEFAULT_SSR_MODE, } as const; const DEFAULT_DYNAMIC_IMPORT_CONFIG: Required = { diff --git a/packages/@lwc/perf-benchmarks-components/rollup.config.mjs b/packages/@lwc/perf-benchmarks-components/rollup.config.mjs index 9c65338e73..d1dff840df 100644 --- a/packages/@lwc/perf-benchmarks-components/rollup.config.mjs +++ b/packages/@lwc/perf-benchmarks-components/rollup.config.mjs @@ -16,8 +16,6 @@ const __dirname = path.dirname(fileURLToPath(import.meta.url)); const { tmpDir, styledComponents } = generateStyledComponents(); -const SSR_MODE = 'asyncYield'; - const ENGINE_TYPE_TO_LWC_IMPORT = { dom: '@lwc/engine-dom', server: '@lwc/engine-server', @@ -36,7 +34,6 @@ function createConfig(componentFile, engineType) { rootDir, experimentalComplexExpressions: true, targetSSR: engineType === 'ssr', - ssrMode: SSR_MODE, }), replace({ preventAssignment: true, diff --git a/packages/@lwc/perf-benchmarks/src/__benchmarks__/ssr/slots/light-slot-create-container-5k.benchmark.js b/packages/@lwc/perf-benchmarks/src/__benchmarks__/ssr/slots/light-slot-create-container-5k.benchmark.js index c987fcb841..9eb0fafefe 100644 --- a/packages/@lwc/perf-benchmarks/src/__benchmarks__/ssr/slots/light-slot-create-container-5k.benchmark.js +++ b/packages/@lwc/perf-benchmarks/src/__benchmarks__/ssr/slots/light-slot-create-container-5k.benchmark.js @@ -2,7 +2,6 @@ import { renderComponent } from '@lwc/ssr-runtime'; import SlotUsage from '@lwc/perf-benchmarks-components/dist/ssr/benchmark/slotUsageComponentLight/slotUsageComponentLight.js'; import Store from '@lwc/perf-benchmarks-components/dist/ssr/benchmark/store/store.js'; -const SSR_MODE = 'asyncYield'; const NUMBER_OF_ROWS = 5000; benchmark(`ssr/slot/light/create/5k`, () => { @@ -24,6 +23,6 @@ benchmark(`ssr/slot/light/create/5k`, () => { titleOfComponentWithSlot: 'Component that receives a slot', rowsOfComponentWithSlot: rowsOfComponentWithSlot, }; - return renderComponent('benchmark-slot-usage-component-light', SlotUsage, props, SSR_MODE); + return renderComponent('benchmark-slot-usage-component-light', SlotUsage, props); }); }); diff --git a/packages/@lwc/perf-benchmarks/src/__benchmarks__/ssr/slots/slot-create-container-5k.benchmark.js b/packages/@lwc/perf-benchmarks/src/__benchmarks__/ssr/slots/slot-create-container-5k.benchmark.js index ad9cc582a9..37e692026c 100644 --- a/packages/@lwc/perf-benchmarks/src/__benchmarks__/ssr/slots/slot-create-container-5k.benchmark.js +++ b/packages/@lwc/perf-benchmarks/src/__benchmarks__/ssr/slots/slot-create-container-5k.benchmark.js @@ -2,7 +2,6 @@ import { renderComponent } from '@lwc/ssr-runtime'; import SlotUsage from '@lwc/perf-benchmarks-components/dist/ssr/benchmark/slotUsageComponent/slotUsageComponent.js'; import Store from '@lwc/perf-benchmarks-components/dist/ssr/benchmark/store/store.js'; -const SSR_MODE = 'asyncYield'; const NUMBER_OF_ROWS = 5000; benchmark(`ssr/slot/shadow/create/5k`, () => { @@ -24,6 +23,6 @@ benchmark(`ssr/slot/shadow/create/5k`, () => { titleOfComponentWithSlot: 'Component that receives a slot', rowsOfComponentWithSlot: rowsOfComponentWithSlot, }; - return renderComponent('benchmark-slot-usage-component', SlotUsage, props, SSR_MODE); + return renderComponent('benchmark-slot-usage-component', SlotUsage, props); }); }); diff --git a/packages/@lwc/perf-benchmarks/src/__benchmarks__/ssr/table/table-render-10k.benchmark.js b/packages/@lwc/perf-benchmarks/src/__benchmarks__/ssr/table/table-render-10k.benchmark.js index 68ade808ae..de185fba1a 100644 --- a/packages/@lwc/perf-benchmarks/src/__benchmarks__/ssr/table/table-render-10k.benchmark.js +++ b/packages/@lwc/perf-benchmarks/src/__benchmarks__/ssr/table/table-render-10k.benchmark.js @@ -10,20 +10,13 @@ import { renderComponent } from '@lwc/ssr-runtime'; import Table from '@lwc/perf-benchmarks-components/dist/ssr/benchmark/table/table.js'; import Store from '@lwc/perf-benchmarks-components/dist/ssr/benchmark/store/store.js'; -const SSR_MODE = 'asyncYield'; - benchmark(`ssr/table-v2/render/10k`, () => { run(() => { const store = new Store(); store.runLots(); - return renderComponent( - 'benchmark-table', - Table, - { - rows: store.data, - }, - SSR_MODE - ); + return renderComponent('benchmark-table', Table, { + rows: store.data, + }); }); }); diff --git a/packages/@lwc/perf-benchmarks/src/__benchmarks__/ssr/table/tablecmp-render-10k.benchmark.js b/packages/@lwc/perf-benchmarks/src/__benchmarks__/ssr/table/tablecmp-render-10k.benchmark.js index 70c4b8594c..30c00ef99e 100644 --- a/packages/@lwc/perf-benchmarks/src/__benchmarks__/ssr/table/tablecmp-render-10k.benchmark.js +++ b/packages/@lwc/perf-benchmarks/src/__benchmarks__/ssr/table/tablecmp-render-10k.benchmark.js @@ -10,20 +10,13 @@ import { renderComponent } from '@lwc/ssr-runtime'; import Table from '@lwc/perf-benchmarks-components/dist/ssr/benchmark/tableComponent/tableComponent.js'; import Store from '@lwc/perf-benchmarks-components/dist/ssr/benchmark/store/store.js'; -const SSR_MODE = 'asyncYield'; - benchmark(`ssr/table-component/render/10k`, () => { run(() => { const store = new Store(); store.runLots(); - return renderComponent( - 'benchmark-table', - Table, - { - rows: store.data, - }, - SSR_MODE - ); + return renderComponent('benchmark-table', Table, { + rows: store.data, + }); }); }); diff --git a/packages/@lwc/perf-benchmarks/src/utils/styledComponentSsrBenchmark.js b/packages/@lwc/perf-benchmarks/src/utils/styledComponentSsrBenchmark.js index 4767016b48..208d0d0424 100644 --- a/packages/@lwc/perf-benchmarks/src/utils/styledComponentSsrBenchmark.js +++ b/packages/@lwc/perf-benchmarks/src/utils/styledComponentSsrBenchmark.js @@ -1,7 +1,5 @@ import { renderComponent } from '@lwc/ssr-runtime'; -const SSR_MODE = 'asyncYield'; - // Generic benchmark for styled components, SSR-flavored! export function styledComponentSsrBenchmark( name, @@ -17,8 +15,7 @@ export function styledComponentSsrBenchmark( await renderComponent( isArray ? `styled-component${i}` : 'styled-component', isArray ? componentOrComponents[i] : componentOrComponents, - {}, - SSR_MODE + {} ); } }); diff --git a/packages/@lwc/shared/src/index.ts b/packages/@lwc/shared/src/index.ts index 5447d335d9..8259705257 100644 --- a/packages/@lwc/shared/src/index.ts +++ b/packages/@lwc/shared/src/index.ts @@ -21,5 +21,6 @@ export * from './static-part-tokens'; export * from './style'; export * from './signals'; export * from './custom-element'; +export * from './ssr'; export { assert }; diff --git a/packages/@lwc/shared/src/ssr.ts b/packages/@lwc/shared/src/ssr.ts new file mode 100644 index 0000000000..69266f1c60 --- /dev/null +++ b/packages/@lwc/shared/src/ssr.ts @@ -0,0 +1,7 @@ +/* + * 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 + */ +export const DEFAULT_SSR_MODE = 'sync'; diff --git a/packages/@lwc/ssr-compiler/src/__tests__/fixtures.spec.ts b/packages/@lwc/ssr-compiler/src/__tests__/fixtures.spec.ts index d887e22c9f..f69359fecf 100644 --- a/packages/@lwc/ssr-compiler/src/__tests__/fixtures.spec.ts +++ b/packages/@lwc/ssr-compiler/src/__tests__/fixtures.spec.ts @@ -12,6 +12,7 @@ import lwcRollupPlugin from '@lwc/rollup-plugin'; import { FeatureFlagName } from '@lwc/features/dist/types'; import { testFixtureDir, formatHTML } from '@lwc/test-utils-lwc-internals'; import { serverSideRenderComponent } from '@lwc/ssr-runtime'; +import { DEFAULT_SSR_MODE } from '@lwc/shared'; import { expectedFailures } from './utils/expected-failures'; import type { CompilationMode } from '../index'; @@ -38,7 +39,7 @@ vi.mock('@lwc/ssr-runtime', async () => { return runtime; }); -const SSR_MODE: CompilationMode = 'asyncYield'; +const SSR_MODE: CompilationMode = DEFAULT_SSR_MODE; async function compileFixture({ input, dirname }: { input: string; dirname: string }) { const modulesDir = path.resolve(dirname, './modules'); 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 7411b5b973..daa540dc44 100644 --- a/packages/@lwc/ssr-compiler/src/__tests__/utils/expected-failures.ts +++ b/packages/@lwc/ssr-compiler/src/__tests__/utils/expected-failures.ts @@ -34,7 +34,6 @@ export const expectedFailures = new Set([ 'empty-text-with-comments-non-static-optimized/index.js', 'if-conditional-slot-content/index.js', 'known-boolean-attributes/default-def-html-attributes/static-on-component/index.js', - 'rehydration/index.js', 'render-dynamic-value/index.js', 'scoped-slots/advanced/index.js', 'scoped-slots/expression/index.js', diff --git a/packages/@lwc/ssr-compiler/src/compile-template/transformers/component.ts b/packages/@lwc/ssr-compiler/src/compile-template/transformers/component.ts index 85dc3cfb20..b95bdc1180 100644 --- a/packages/@lwc/ssr-compiler/src/compile-template/transformers/component.ts +++ b/packages/@lwc/ssr-compiler/src/compile-template/transformers/component.ts @@ -29,7 +29,7 @@ const bYieldFromChildGenerator = esTemplateWithYield` // The 'instance' variable is shadowed here so that a contextful relationship // is established between components rendered in slotted content & the "parent" // component that contains the . - shadow: async function* (instance) { + shadow: async function* generateSlottedContent(instance) { ${/* shadow slot content */ is.statement} } }; @@ -59,8 +59,11 @@ const bYieldFromChildGenerator = 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 bAddContent = esTemplate` - addContent(${/* slot name */ is.expression} ?? "", async function* (${ + addContent(${/* slot name */ is.expression} ?? "", async function* generateSlottedContent(${ /* scoped slot data variable */ isNullableOf(is.identifier) }) { // FIXME: make validation work again diff --git a/packages/@lwc/ssr-compiler/src/index.ts b/packages/@lwc/ssr-compiler/src/index.ts index 2a6211e7dd..d4551cc32d 100644 --- a/packages/@lwc/ssr-compiler/src/index.ts +++ b/packages/@lwc/ssr-compiler/src/index.ts @@ -5,7 +5,7 @@ * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT */ -import { generateCustomElementTagName } from '@lwc/shared'; +import { DEFAULT_SSR_MODE, generateCustomElementTagName } from '@lwc/shared'; import compileJS from './compile-js'; import compileTemplate from './compile-template'; import type { CompilationMode, TransformOptions } from './shared'; @@ -21,7 +21,7 @@ export function compileComponentForSSR( src: string, filename: string, options: TransformOptions, - mode: CompilationMode = 'asyncYield' + mode: CompilationMode = DEFAULT_SSR_MODE ): CompilationResult { const tagName = generateCustomElementTagName(options.namespace, options.name); const { code } = compileJS(src, filename, tagName, mode); @@ -32,7 +32,7 @@ export function compileTemplateForSSR( src: string, filename: string, options: TransformOptions, - mode: CompilationMode = 'asyncYield' + mode: CompilationMode = DEFAULT_SSR_MODE ): CompilationResult { const { code } = compileTemplate(src, filename, options, mode); return { code, map: undefined }; diff --git a/packages/@lwc/ssr-compiler/src/transmogrify.ts b/packages/@lwc/ssr-compiler/src/transmogrify.ts index 7bbb6767a7..952095992c 100644 --- a/packages/@lwc/ssr-compiler/src/transmogrify.ts +++ b/packages/@lwc/ssr-compiler/src/transmogrify.ts @@ -6,8 +6,9 @@ */ import { traverse, builders as b, type NodePath } from 'estree-toolkit'; import { produce } from 'immer'; -import type { Node } from 'estree'; +import type { FunctionDeclaration, FunctionExpression, Node } from 'estree'; import type { Program as EsProgram } from 'estree'; +import type { Node as EstreeToolkitNode } from 'estree-toolkit/dist/helpers'; export type TransmogrificationMode = 'sync' | 'async'; @@ -20,14 +21,19 @@ export type Visitors = Parameters const EMIT_IDENT = b.identifier('$$emit'); // Rollup may rename variables to prevent shadowing. When it does, it uses the format `foo$0`, `foo$1`, etc. const TMPL_FN_PATTERN = /tmpl($\d+)?/; -const GEN_MARKUP_PATTERN = /generateMarkup($\d+)?/; +const GEN_MARKUP_OR_GEN_SLOTTED_CONTENT_PATTERN = + /(?:generateMarkup|generateSlottedContent)($\d+)?/; const isWithinFn = (pattern: RegExp, nodePath: NodePath): boolean => { const { node } = nodePath; if (!node) { return false; } - if (node.type === 'FunctionDeclaration' && pattern.test(node.id.name)) { + if ( + (node.type === 'FunctionDeclaration' || node.type === 'FunctionExpression') && + node.id && + pattern.test(node.id.name) + ) { return true; } if (nodePath.parentPath) { @@ -36,23 +42,32 @@ const isWithinFn = (pattern: RegExp, nodePath: NodePath): boolean => { return false; }; -const visitors: Visitors = { - FunctionDeclaration(path, state) { - const { node } = path; - if (!node?.async || !node?.generator) { - return; - } +function transformFunction( + path: NodePath, + state: TransmogrificationState +): undefined { + const { node } = path; + if (!node?.async || !node?.generator) { + return; + } - // Component authors might conceivably use async generator functions in their own code. Therefore, - // when traversing & transforming written+generated code, we need to disambiguate generated async - // generator functions from those that were written by the component author. - if (!isWithinFn(GEN_MARKUP_PATTERN, path) && !isWithinFn(TMPL_FN_PATTERN, path)) { - return; - } - node.generator = false; - node.async = state.mode === 'async'; - node.params.unshift(EMIT_IDENT); - }, + // Component authors might conceivably use async generator functions in their own code. Therefore, + // when traversing & transforming written+generated code, we need to disambiguate generated async + // generator functions from those that were written by the component author. + if ( + !isWithinFn(GEN_MARKUP_OR_GEN_SLOTTED_CONTENT_PATTERN, path) && + !isWithinFn(TMPL_FN_PATTERN, path) + ) { + return; + } + node.generator = false; + node.async = state.mode === 'async'; + node.params.unshift(EMIT_IDENT); +} + +const visitors: Visitors = { + FunctionDeclaration: transformFunction, + FunctionExpression: transformFunction, YieldExpression(path, state) { const { node } = path; if (!node) { @@ -62,7 +77,10 @@ const visitors: Visitors = { // Component authors might conceivably use generator functions within their own code. Therefore, // when traversing & transforming written+generated code, we need to disambiguate generated yield // expressions from those that were written by the component author. - if (!isWithinFn(TMPL_FN_PATTERN, path) && !isWithinFn(GEN_MARKUP_PATTERN, path)) { + if ( + !isWithinFn(TMPL_FN_PATTERN, path) && + !isWithinFn(GEN_MARKUP_OR_GEN_SLOTTED_CONTENT_PATTERN, path) + ) { return; } diff --git a/packages/@lwc/ssr-runtime/src/render.ts b/packages/@lwc/ssr-runtime/src/render.ts index 2e137b0dab..06167da635 100644 --- a/packages/@lwc/ssr-runtime/src/render.ts +++ b/packages/@lwc/ssr-runtime/src/render.ts @@ -4,7 +4,7 @@ * SPDX-License-Identifier: MIT * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT */ -import { getOwnPropertyNames, isNull, isString, isUndefined } from '@lwc/shared'; +import { getOwnPropertyNames, isNull, isString, isUndefined, DEFAULT_SSR_MODE } from '@lwc/shared'; import { mutationTracker } from './mutation-tracker'; import { LightningElement, @@ -143,7 +143,7 @@ export async function serverSideRenderComponent( tagName: string, Component: GenerateMarkupFnVariants | ComponentWithGenerateMarkup, props: Properties = {}, - mode: 'asyncYield' | 'async' | 'sync' = 'asyncYield' + mode: 'asyncYield' | 'async' | 'sync' = DEFAULT_SSR_MODE ): Promise { if (typeof tagName !== 'string') { throw new Error(`tagName must be a string, found: ${tagName}`);