From b47e18c9b9f65b0d3ac94de774ac5a258a97cafb Mon Sep 17 00:00:00 2001 From: Nolan Lawson Date: Fri, 15 Nov 2024 12:40:35 -0800 Subject: [PATCH] fix(ssr): remove unused imports (#4873) --- .../src/__tests__/fixtures.spec.ts | 15 ++----- .../@lwc/ssr-compiler/src/compile-js/index.ts | 3 ++ .../src/compile-template/index.ts | 3 ++ .../compile-template/transformers/element.ts | 12 +++--- .../@lwc/ssr-compiler/src/optimize-imports.ts | 42 +++++++++++++++++++ 5 files changed, 56 insertions(+), 19 deletions(-) create mode 100644 packages/@lwc/ssr-compiler/src/optimize-imports.ts diff --git a/packages/@lwc/ssr-compiler/src/__tests__/fixtures.spec.ts b/packages/@lwc/ssr-compiler/src/__tests__/fixtures.spec.ts index 712c400e94..d887e22c9f 100644 --- a/packages/@lwc/ssr-compiler/src/__tests__/fixtures.spec.ts +++ b/packages/@lwc/ssr-compiler/src/__tests__/fixtures.spec.ts @@ -57,19 +57,10 @@ async function compileFixture({ input, dirname }: { input: string; dirname: stri modules: [{ dir: modulesDir }], }), ], - onwarn({ message, code, names }) { - if (code === 'CIRCULAR_DEPENDENCY') { - return; + onwarn({ message, code }) { + if (code !== 'CIRCULAR_DEPENDENCY') { + throw new Error(message); } - // TODO [#4793]: fix unused imports - if (code === 'UNUSED_EXTERNAL_IMPORT') { - const unexpected = new Set(names); - const expected = ['connectContext', 'htmlEscape', 'track']; - expected.forEach((name) => unexpected.delete(name)); - if (unexpected.size === 0) return; - } - - throw new Error(message); }, }); diff --git a/packages/@lwc/ssr-compiler/src/compile-js/index.ts b/packages/@lwc/ssr-compiler/src/compile-js/index.ts index f2ef0e4d15..766097d2b8 100644 --- a/packages/@lwc/ssr-compiler/src/compile-js/index.ts +++ b/packages/@lwc/ssr-compiler/src/compile-js/index.ts @@ -10,6 +10,7 @@ import { traverse, builders as b, is } from 'estree-toolkit'; import { parseModule } from 'meriyah'; import { transmogrify } from '../transmogrify'; +import { optimizeImports } from '../optimize-imports'; import { replaceLwcImport } from './lwc-import'; import { catalogTmplImport } from './catalog-tmpls'; import { catalogStaticStylesheets, catalogAndReplaceStyleImports } from './stylesheets'; @@ -193,6 +194,8 @@ export default function compileJS( addGenerateMarkupExport(ast, state, tagName, filename); assignGenerateMarkupToComponent(ast, state); + ast = optimizeImports(ast); + if (compilationMode === 'async' || compilationMode === 'sync') { ast = transmogrify(ast, compilationMode); } diff --git a/packages/@lwc/ssr-compiler/src/compile-template/index.ts b/packages/@lwc/ssr-compiler/src/compile-template/index.ts index 051e58d3d7..21c67dbb4c 100644 --- a/packages/@lwc/ssr-compiler/src/compile-template/index.ts +++ b/packages/@lwc/ssr-compiler/src/compile-template/index.ts @@ -14,6 +14,7 @@ import { getStylesheetImports } from '../compile-js/stylesheets'; import { addScopeTokenDeclarations } from '../compile-js/stylesheet-scope-token'; import { transmogrify } from '../transmogrify'; import { bImportDeclaration } from '../estree/builders'; +import { optimizeImports } from '../optimize-imports'; import { optimizeAdjacentYieldStmts } from './shared'; import { templateIrToEsTree } from './ir-to-es'; import type { ExportDefaultDeclaration as EsExportDefaultDeclaration } from 'estree'; @@ -110,6 +111,8 @@ export default function compileTemplate( const stylesheetImports = getStylesheetImports(filename); program.body.unshift(...stylesheetImports); + program = optimizeImports(program); + if (compilationMode === 'async' || compilationMode === 'sync') { program = transmogrify(program, compilationMode); } diff --git a/packages/@lwc/ssr-compiler/src/compile-template/transformers/element.ts b/packages/@lwc/ssr-compiler/src/compile-template/transformers/element.ts index 2425dce070..f31ca5c6ae 100644 --- a/packages/@lwc/ssr-compiler/src/compile-template/transformers/element.ts +++ b/packages/@lwc/ssr-compiler/src/compile-template/transformers/element.ts @@ -141,7 +141,6 @@ function yieldAttrOrPropLiveValue( value: IrExpression | BinaryExpression, cxt: TransformerContext ): EsStatement[] { - cxt.hoist(bImportHtmlEscape(), importHtmlEscapeKey); const isHtmlBooleanAttr = isBooleanAttribute(name, elementName); const scopedExpression = getScopedExpression(value as EsExpression, cxt); return [bConditionalLiveYield(b.literal(name), scopedExpression, b.literal(isHtmlBooleanAttr))]; @@ -205,12 +204,9 @@ export const Element: Transformer = fu result = yieldAttrOrPropLiveValue(node.name, name, value, cxt); } - if (result.length > 0) { - // actually yielded something - cxt.hoist(bImportHtmlEscape(), importHtmlEscapeKey); - if (name === 'class') { - hasClassAttribute = true; - } + if (result.length > 0 && name === 'class') { + // actually yielded a class attribute value + hasClassAttribute = true; } return result; @@ -235,6 +231,8 @@ export const Element: Transformer = fu childContent = []; } + cxt.hoist(bImportHtmlEscape(), importHtmlEscapeKey); + return [ bYield(b.literal(`<${node.name}`)), // If we haven't already prefixed the scope token to an existing class, add an explicit class here diff --git a/packages/@lwc/ssr-compiler/src/optimize-imports.ts b/packages/@lwc/ssr-compiler/src/optimize-imports.ts new file mode 100644 index 0000000000..d444a23969 --- /dev/null +++ b/packages/@lwc/ssr-compiler/src/optimize-imports.ts @@ -0,0 +1,42 @@ +/* + * 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 { produce } from 'immer'; +import { traverse } from 'estree-toolkit'; +import { Visitors } from './transmogrify'; +import type { Program as EsProgram } from 'estree'; + +const visitors: Visitors = { + $: { + scope: true, + }, + ImportDeclaration(path) { + const { node, scope } = path; + if (!node || !scope) { + return; + } + if (node.source.type === 'Literal' && node.source.value === '@lwc/ssr-runtime') { + node.specifiers = node.specifiers.filter( + (specifier) => + // There shouldn't be any default imports anyway + specifier.type === 'ImportSpecifier' && + // If there are references, then the import is used. Note that an import will _always_ + // have a binding (of type "module"), but it may not have references. + scope.getBinding(specifier.local.name)?.references.length + ); + } + }, +}; + +/** + * Remove any unused imports from `@lwc/ssr-runtime`. + * This avoids any warnings from Rollup about unused imports, and avoids us needing + * to manually track what's imported during AST generation. + * @param compiledComponentAst + */ +export function optimizeImports(compiledComponentAst: EsProgram): EsProgram { + return produce(compiledComponentAst, (astDraft) => traverse(astDraft, visitors)); +}