diff --git a/code/lib/cli-storybook/src/automigrate/fixes/angular-vite-remove-compodoc.ts b/code/lib/cli-storybook/src/automigrate/fixes/angular-vite-remove-compodoc.ts index 82ae6dc4f023..8f39f5121a3b 100644 --- a/code/lib/cli-storybook/src/automigrate/fixes/angular-vite-remove-compodoc.ts +++ b/code/lib/cli-storybook/src/automigrate/fixes/angular-vite-remove-compodoc.ts @@ -412,15 +412,15 @@ const manualRemovalHint = (previewConfigPath: string, reason: string): void => ); const removePreviewWiring = (preview: ConfigFile, previewConfigPath: string): void => { - const reason = removeTopLevelCall(preview._ast.program, { + const result = removeTopLevelCall(preview._ast.program, { callee: SET_COMPODOC_JSON, calleeSources: new Set([ADDON_DOCS_ANGULAR]), }); - if (reason) { + if (!result.removed) { manualRemovalHint( previewConfigPath, countReferences(preview._ast.program, SET_COMPODOC_JSON) > 0 - ? reason + ? result.reason : `no ${SET_COMPODOC_JSON} call is visible here, only a documentation.json import` ); return; diff --git a/code/lib/cli-storybook/src/automigrate/fixes/web-components-runtime-manifest.test.ts b/code/lib/cli-storybook/src/automigrate/fixes/web-components-runtime-manifest.test.ts index ec3914b222b6..abdd7e4f9fce 100644 --- a/code/lib/cli-storybook/src/automigrate/fixes/web-components-runtime-manifest.test.ts +++ b/code/lib/cli-storybook/src/automigrate/fixes/web-components-runtime-manifest.test.ts @@ -67,6 +67,7 @@ const runWith = async ( result: { setterNames: ['setCustomElementsManifest'], manifestPath: '../custom-elements.json', + manifestFiles: ['/project/custom-elements.json'], ...result, }, mainConfigPath: MAIN, @@ -138,6 +139,7 @@ describe('check', (): void => { expected: { setterNames: ['setCustomElementsManifest'], manifestPath: '../custom-elements.json', + manifestFiles: ['/project/custom-elements.json'], }, }, { @@ -149,6 +151,7 @@ describe('check', (): void => { expected: { setterNames: ['setCustomElementsManifest'], manifestPath: '../custom-elements.json', + manifestFiles: ['/project/custom-elements.json'], }, }, { @@ -166,6 +169,7 @@ describe('check', (): void => { expected: { setterNames: ['setCustomElementsManifest'], manifestPath: '../custom-elements.json', + manifestFiles: ['/project/custom-elements.json'], }, }, { @@ -182,6 +186,7 @@ describe('check', (): void => { expected: { setterNames: ['setCustomElements'], manifestPath: '../custom-elements.json', + manifestFiles: ['/project/custom-elements.json'], }, }, { @@ -235,6 +240,7 @@ describe('check', (): void => { expected: { setterNames: ['setCustomElementsManifest'], manifestPath: null, + manifestFiles: ['/project/custom-elements.json'], }, }, { @@ -251,6 +257,7 @@ describe('check', (): void => { expected: { setterNames: ['setCustomElementsManifest'], manifestPath: '../custom-elements.json', + manifestFiles: ['/project/custom-elements.json'], }, }, { @@ -268,6 +275,43 @@ describe('check', (): void => { expected: { setterNames: ['setCustomElementsManifest'], manifestPath: '../custom-elements.json', + manifestFiles: ['/project/custom-elements.json'], + }, + }, + { + name: 'uses the workspace manifest when the setter argument wraps the import', + files: { + '/project/custom-elements.json': CEM, + [PREVIEW]: dedent` + import { setCustomElementsManifest } from "@storybook/web-components"; + import customElements from "../custom-elements.json"; + + setCustomElementsManifest(format(customElements)); + `, + }, + expected: { + setterNames: ['setCustomElementsManifest'], + manifestPath: '../custom-elements.json', + manifestFiles: ['/project/custom-elements.json'], + }, + }, + { + name: 'uses package.json customElements when the setter reads a derived variable', + files: { + '/project/custom-elements.json': CEM, + [PACKAGE_JSON]: JSON.stringify({ customElements: 'custom-elements.json' }), + [PREVIEW]: dedent` + import { setCustomElementsManifest } from "@storybook/web-components"; + import customElements from "../custom-elements.json"; + + const m = format(customElements); + setCustomElementsManifest(m); + `, + }, + expected: { + setterNames: ['setCustomElementsManifest'], + manifestPath: null, + manifestFiles: ['/project/custom-elements.json'], }, }, { @@ -310,6 +354,7 @@ describe('check', (): void => { expected: { setterNames: ['setCustomElementsManifest'], manifestPath: '../preview-elements.json', + manifestFiles: ['/project/preview-elements.json'], }, }, ]; @@ -337,12 +382,14 @@ describe('check', (): void => { const result = await checkFix(webComponentsRuntimeManifest, checkOptions({})); expect(result).toBeNull(); - expect(vi.mocked(logger.warn)).toHaveBeenCalledWith( - 'Left the setCustomElementsManifest() call in /project/.storybook/preview.ts alone: ' + - '/project/custom-elements.json is not a readable Custom Elements Manifest. Generate ' + - 'one with @custom-elements-manifest/analyzer, then rerun "storybook automigrate ' + - 'web-components-runtime-manifest".' - ); + expect(vi.mocked(logger.warn).mock.calls).toMatchInlineSnapshot(` + [ + [ + "Left the setCustomElementsManifest() call in /project/.storybook/preview.ts alone: /project/custom-elements.json is not a readable Custom Elements Manifest. + Generate one with @custom-elements-manifest/analyzer, then rerun "storybook automigrate web-components-runtime-manifest".", + ], + ] + `); }); it('returns null when package.json customElements includes a WCA manifest', async (): Promise => { @@ -381,12 +428,14 @@ describe('check', (): void => { const result = await checkFix(webComponentsRuntimeManifest, checkOptions({})); expect(result).toBeNull(); - expect(vi.mocked(logger.warn)).toHaveBeenCalledWith( - 'Left the setCustomElementsManifest() call in /project/.storybook/preview.ts alone: ' + - '/project/custom-elements.json is not a readable Custom Elements Manifest. Generate ' + - 'one with @custom-elements-manifest/analyzer, then rerun "storybook automigrate ' + - 'web-components-runtime-manifest".' - ); + expect(vi.mocked(logger.warn).mock.calls).toMatchInlineSnapshot(` + [ + [ + "Left the setCustomElementsManifest() call in /project/.storybook/preview.ts alone: /project/custom-elements.json is not a readable Custom Elements Manifest. + Generate one with @custom-elements-manifest/analyzer, then rerun "storybook automigrate web-components-runtime-manifest".", + ], + ] + `); }); }); @@ -477,6 +526,7 @@ describe('run', (): void => { expect(vol.readFileSync(PREVIEW, 'utf8')).toMatchInlineSnapshot(` "export const parameters = { controls: { expanded: true } };" `); + expect(logger.warn).not.toHaveBeenCalled(); }); it('strips an inline object call and the setter specifier', async (): Promise => { @@ -497,6 +547,28 @@ describe('run', (): void => { `); }); + it('does not warn about a read JSON import of another file', async (): Promise => { + vol.fromNestedJSON({ + [PREVIEW]: dedent` + import { setCustomElementsManifest } from "@storybook/web-components"; + import tokens from "../tokens.json"; + + setCustomElementsManifest({}); + + export const tokenCount = tokens.length; + `, + }); + + await runWith(); + + expect(vol.readFileSync(PREVIEW, 'utf8')).toMatchInlineSnapshot(` + "import tokens from "../tokens.json"; + + export const tokenCount = tokens.length;" + `); + expect(logger.warn).not.toHaveBeenCalled(); + }); + it('strips both setters when the preview calls both', async (): Promise => { vol.fromNestedJSON({ [PREVIEW]: dedent` @@ -518,6 +590,60 @@ describe('run', (): void => { `); }); + it('does not warn when both setters read the same JSON import', async (): Promise => { + vol.fromNestedJSON({ + [PREVIEW]: dedent` + import { setCustomElements, setCustomElementsManifest } from "@storybook/web-components"; + import customElements from "../custom-elements.json"; + + setCustomElementsManifest(customElements); + setCustomElements(customElements); + + export const parameters = {}; + `, + }); + + await runWith({ setterNames: ['setCustomElementsManifest', 'setCustomElements'] }); + + expect(vol.readFileSync(PREVIEW, 'utf8')).toMatchInlineSnapshot(` + "export const parameters = {};" + `); + expect(logger.warn).not.toHaveBeenCalled(); + }); + + it('only hints at manual removal when the other setter keeps the JSON import', async (): Promise => { + vol.fromNestedJSON({ + [PREVIEW]: dedent` + import { setCustomElements, setCustomElementsManifest } from "@storybook/web-components"; + import customElements from "../custom-elements.json"; + + setCustomElementsManifest(customElements); + setCustomElements(customElements); + + export const configure = setCustomElements; + `, + }); + + await runWith({ setterNames: ['setCustomElementsManifest', 'setCustomElements'] }); + + expect(vol.readFileSync(PREVIEW, 'utf8')).toMatchInlineSnapshot(` + "import { setCustomElements } from "@storybook/web-components"; + import customElements from "../custom-elements.json"; + + setCustomElements(customElements); + + export const configure = setCustomElements;" + `); + expect(vi.mocked(logger.warn).mock.calls).toMatchInlineSnapshot(` + [ + [ + "Left the setCustomElements wiring in /project/.storybook/preview.ts alone: setCustomElements is still used elsewhere. + Remove the runtime call by hand when convenient.", + ], + ] + `); + }); + it('leaves helper-wrapped preview calls alone while migrating main', async (): Promise => { const preview = dedent` import { setCustomElementsManifest } from "@storybook/web-components"; @@ -585,6 +711,162 @@ describe('run', (): void => { export const moduleCount = customElements.modules.length;" `); + expect(vi.mocked(logger.warn).mock.calls).toMatchInlineSnapshot(` + [ + [ + "/project/.storybook/preview.ts still imports ../custom-elements.json for: \`export const moduleCount = customElements.modules.length\`. + Docgen now reads the manifest from disk, so changes made to it in the preview no longer apply, and the import makes the preview build fail when the manifest is invalid. + Move manifest transforms into your manifest generator (for example an analyzer plugin), then remove the import.", + ], + ] + `); + }); + + it('warns only about top-level readers of a surviving JSON import', async (): Promise => { + vol.fromNestedJSON({ + [PREVIEW]: dedent` + import { setCustomElementsManifest } from "@storybook/web-components"; + import customElements from "../custom-elements.json"; + + WebComponentFormatter(customElements); + setCustomElementsManifest(customElements); + + function WebComponentFormatter(customElements) { + for (const module of customElements.modules) { + module.path = module.path.replace(/\\.ts$/, '.js'); + } + } + `, + }); + + await runWith(); + + expect(vol.readFileSync(PREVIEW, 'utf8')).toMatchInlineSnapshot(` + "import customElements from "../custom-elements.json"; + + WebComponentFormatter(customElements); + + function WebComponentFormatter(customElements) { + for (const module of customElements.modules) { + module.path = module.path.replace(/\\.ts$/, '.js'); + } + }" + `); + expect(vi.mocked(logger.warn).mock.calls).toMatchInlineSnapshot(` + [ + [ + "/project/.storybook/preview.ts still imports ../custom-elements.json for: \`WebComponentFormatter(customElements)\`. + Docgen now reads the manifest from disk, so changes made to it in the preview no longer apply, and the import makes the preview build fail when the manifest is invalid. + Move manifest transforms into your manifest generator (for example an analyzer plugin), then remove the import.", + ], + ] + `); + }); + + it('drops the JSON import when only a shadowing parameter shares its name', async (): Promise => { + vol.fromNestedJSON({ + [PREVIEW]: dedent` + import { setCustomElementsManifest } from "@storybook/web-components"; + import customElements from "../custom-elements.json"; + + setCustomElementsManifest(customElements); + + export function count(customElements) { return customElements.modules.length; } + `, + }); + + await runWith(); + + expect(vol.readFileSync(PREVIEW, 'utf8')).toMatchInlineSnapshot(` + "export function count(customElements) { return customElements.modules.length; }" + `); + expect(logger.warn).not.toHaveBeenCalled(); + }); + + it('prints the reader code without its leading comment', async (): Promise => { + vol.fromNestedJSON({ + [PREVIEW]: dedent` + import { setCustomElementsManifest } from "@storybook/web-components"; + import customElements from "../custom-elements.json"; + + // Fix the paths before docs read them + WebComponentFormatter(customElements); + setCustomElementsManifest(customElements); + `, + }); + + await runWith(); + + expect(vi.mocked(logger.warn).mock.calls[0][0]).toContain( + 'for: `WebComponentFormatter(customElements)`.' + ); + }); + + it('drops a manifest import that only fed a wrapped setter argument', async (): Promise => { + vol.fromNestedJSON({ + [PREVIEW]: dedent` + import { setCustomElementsManifest } from "@storybook/web-components"; + import customElements from "../custom-elements.json"; + + setCustomElementsManifest(format(customElements)); + + export const parameters = {}; + `, + }); + + await runWith({ manifestPath: null }); + + expect(vol.readFileSync(PREVIEW, 'utf8')).toMatchInlineSnapshot(` + "export const parameters = {};" + `); + expect(logger.warn).not.toHaveBeenCalled(); + }); + + it('warns about a manifest import that feeds the setter through a variable', async (): Promise => { + vol.fromNestedJSON({ + [PREVIEW]: dedent` + import { setCustomElementsManifest } from "@storybook/web-components"; + import customElements from "../custom-elements.json"; + + const m = format(customElements); + setCustomElementsManifest(m); + `, + }); + + await runWith({ manifestPath: null }); + + expect(vol.readFileSync(PREVIEW, 'utf8')).toMatchInlineSnapshot(` + "import customElements from "../custom-elements.json"; + + const m = format(customElements);" + `); + expect(vi.mocked(logger.warn).mock.calls).toMatchInlineSnapshot(` + [ + [ + "/project/.storybook/preview.ts still imports ../custom-elements.json for: \`const m = format(customElements)\`. + Docgen now reads the manifest from disk, so changes made to it in the preview no longer apply, and the import makes the preview build fail when the manifest is invalid. + Move manifest transforms into your manifest generator (for example an analyzer plugin), then remove the import.", + ], + ] + `); + }); + + it('leaves an unused JSON import of another file alone', async (): Promise => { + vol.fromNestedJSON({ + [PREVIEW]: dedent` + import { setCustomElementsManifest } from "@storybook/web-components"; + import tokens from "../tokens.json"; + + setCustomElementsManifest({}); + `, + }); + + await runWith({ manifestPath: null }); + + expect(vol.readFileSync(PREVIEW, 'utf8')).toMatchInlineSnapshot(` + "import tokens from "../tokens.json";" + `); + expect(logger.warn).not.toHaveBeenCalled(); }); it('leaves the preview alone and warns when the setter is also referenced elsewhere', async (): Promise => { diff --git a/code/lib/cli-storybook/src/automigrate/fixes/web-components-runtime-manifest.ts b/code/lib/cli-storybook/src/automigrate/fixes/web-components-runtime-manifest.ts index 4e7b0575e388..ddd6ae04c687 100644 --- a/code/lib/cli-storybook/src/automigrate/fixes/web-components-runtime-manifest.ts +++ b/code/lib/cli-storybook/src/automigrate/fixes/web-components-runtime-manifest.ts @@ -1,14 +1,21 @@ -import { babelParse, types as t } from 'storybook/internal/babel'; +import { babelParse, generate, types as t } from 'storybook/internal/babel'; import { findFilesUp } from 'storybook/internal/common'; import type { ConfigFile } from 'storybook/internal/csf-tools'; import { logger } from 'storybook/internal/node-logger'; import { SupportedRenderer } from 'storybook/internal/types'; import { dirname, relative, resolve } from 'pathe'; +import { dedent } from 'ts-dedent'; import type { FixFiles } from '../fix-files.ts'; import { getRendererName } from '../helpers/mainConfigFile.ts'; -import { countReferences, removeTopLevelCall, topLevelCalls } from '../helpers/preview-call.ts'; +import { + countReferences, + isTopLevelCall, + removeTopLevelCall, + topLevelCalls, + topLevelReaders, +} from '../helpers/preview-call.ts'; import { findWorkspaceFiles, readJsonFile } from '../helpers/workspace-files.ts'; import type { FixTransform } from '../pipeline.ts'; import type { Fix } from '../types.ts'; @@ -19,6 +26,7 @@ const WEB_COMPONENTS_PACKAGES = new Set([ '@storybook/web-components-vite', ]); const AUTOMIGRATE_COMMAND = 'storybook automigrate web-components-runtime-manifest'; +const MAX_STATEMENT_LENGTH = 80; type SetterName = (typeof SETTER_NAMES)[number]; @@ -29,6 +37,8 @@ export interface WebComponentsRuntimeManifestOptions { setterNames: SetterName[]; /** Manifest path relative to the config dir; `null` when `package.json#customElements` already names it. */ manifestPath: string | null; + /** Absolute paths of the manifests the docgen server will read. */ + manifestFiles: string[]; } export const webComponentsRuntimeManifest: Fix = { @@ -79,6 +89,7 @@ export const webComponentsRuntimeManifest: Fix { - removePreviewWiring(preview, id, result.setterNames); + removePreviewWiring(preview, id, result); }, }, ], @@ -198,11 +209,10 @@ const logInvalidManifestWarning = ( setterName: SetterName, path: string ): void => - logger.warn( - `Left the ${setterName}() call in ${previewConfigPath} alone: ${path} is not a readable ` + - `Custom Elements Manifest. Generate one with @custom-elements-manifest/analyzer, then rerun ` + - `"${AUTOMIGRATE_COMMAND}".` - ); + logger.warn(dedent` + Left the ${setterName}() call in ${previewConfigPath} alone: ${path} is not a readable Custom Elements Manifest. + Generate one with @custom-elements-manifest/analyzer, then rerun "${AUTOMIGRATE_COMMAND}". + `); const findDefaultJsonImportSpecifier = (program: t.Program, name: string): string | null => { for (const node of program.body) { @@ -228,25 +238,103 @@ const logManualRemovalHint = ( setterName: SetterName, reason: string ): void => - logger.warn( - `Left the ${setterName} wiring in ${previewConfigPath} alone: ${reason}. ` + - `Remove the runtime call by hand when convenient.` - ); + logger.warn(dedent` + Left the ${setterName} wiring in ${previewConfigPath} alone: ${reason}. + Remove the runtime call by hand when convenient. + `); + +const printStatement = (node: t.Statement): string => { + const [firstLine] = generate(node, { comments: false }).code.split('\n'); + const line = firstLine.replace(/;$/, ''); + return line.length > MAX_STATEMENT_LENGTH ? `${line.slice(0, MAX_STATEMENT_LENGTH)}...` : line; +}; + +const logSurvivingManifestImport = ( + previewConfigPath: string, + specifier: string, + readers: t.Statement[] +): void => + logger.warn(dedent` + ${previewConfigPath} still imports ${specifier} for: ${readers.map((node) => `\`${printStatement(node)}\``).join(', ')}. + Docgen now reads the manifest from disk, so changes made to it in the preview no longer apply, and the import makes the preview build fail when the manifest is invalid. + Move manifest transforms into your manifest generator (for example an analyzer plugin), then remove the import. + `); + +const findManifestImports = ( + program: t.Program, + previewDir: string, + manifestFiles: string[] +): Map => { + const imports = new Map(); + for (const node of program.body) { + if ( + !t.isImportDeclaration(node) || + !isRelativeJsonSpecifier(node.source.value) || + !manifestFiles.includes(resolve(previewDir, node.source.value)) + ) { + continue; + } + for (const specifier of node.specifiers) { + if (t.isImportDefaultSpecifier(specifier)) { + imports.set(specifier.local.name, node.source.value); + } + } + } + return imports; +}; + +const removeDefaultImport = (program: t.Program, name: string): void => { + program.body = program.body.filter((node) => { + if (!t.isImportDeclaration(node)) { + return true; + } + node.specifiers = node.specifiers.filter( + (specifier) => !(t.isImportDefaultSpecifier(specifier) && specifier.local.name === name) + ); + return node.specifiers.length > 0; + }); +}; const removePreviewWiring = ( preview: ConfigFile, previewConfigPath: string, - setterNames: SetterName[] + { setterNames, manifestFiles }: WebComponentsRuntimeManifestOptions ): void => { + const { program } = preview._ast; + const manifestImports = findManifestImports(program, dirname(previewConfigPath), manifestFiles); + const keptImports = new Map(manifestImports); + let removedAnySetter = false; for (const setterName of setterNames) { - const reason = removeTopLevelCall(preview._ast.program, { + const result = removeTopLevelCall(program, { callee: setterName, calleeSources: WEB_COMPONENTS_PACKAGES, }); - if (reason) { - logManualRemovalHint(previewConfigPath, setterName, reason); + if (!result.removed) { + logManualRemovalHint(previewConfigPath, setterName, result.reason); continue; } + removedAnySetter = true; logger.debug(`Removed the ${setterName} wiring from ${previewConfigPath}`); + for (const name of result.keptArguments) { + const specifier = findDefaultJsonImportSpecifier(program, name); + if (specifier) { + keptImports.set(name, specifier); + } + } + } + if (!removedAnySetter) { + return; + } + + for (const [name, specifier] of keptImports) { + // A setter call left in place already got the manual-removal hint. + const readers = topLevelReaders(program, name).filter( + (node) => !SETTER_NAMES.some((setterName) => isTopLevelCall(node, setterName)) + ); + if (readers.length > 0) { + logSurvivingManifestImport(previewConfigPath, specifier, readers); + } else if (manifestImports.has(name) && countReferences(program, name) === 0) { + removeDefaultImport(program, name); + } } }; diff --git a/code/lib/cli-storybook/src/automigrate/helpers/preview-call.ts b/code/lib/cli-storybook/src/automigrate/helpers/preview-call.ts index e6ffc5f33cdf..d74c51a001c6 100644 --- a/code/lib/cli-storybook/src/automigrate/helpers/preview-call.ts +++ b/code/lib/cli-storybook/src/automigrate/helpers/preview-call.ts @@ -1,59 +1,82 @@ -import { traverse, types as t } from 'storybook/internal/babel'; +import { type NodePath, traverse, types as t } from 'storybook/internal/babel'; + +export type RemoveTopLevelCallResult = + | { removed: false; reason: string } + | { removed: true; keptArguments: string[] }; + +export const isTopLevelCall = (node: t.Statement, callee: string): node is t.ExpressionStatement => + t.isExpressionStatement(node) && + t.isCallExpression(node.expression) && + t.isIdentifier(node.expression.callee, { name: callee }); export const topLevelCalls = (program: t.Program, callee: string): t.ExpressionStatement[] => - program.body.filter( - (node): node is t.ExpressionStatement => - t.isExpressionStatement(node) && - t.isCallExpression(node.expression) && - t.isIdentifier(node.expression.callee, { name: callee }) - ); + program.body.filter((node): node is t.ExpressionStatement => isTopLevelCall(node, callee)); -export const countReferences = (program: t.Program, name: string): number => { - let references = 0; +/** Reads of `name` that resolve to the program-level binding or to a global, skipping shadowed ones. */ +export const referencePaths = (program: t.Program, name: string): NodePath[] => { + const paths: NodePath[] = []; traverse(t.file(program), { Identifier(path): void { if (path.node.name !== name || path.parentPath?.isImportDefaultSpecifier()) { return; } - if (path.isReferencedIdentifier()) { - references += 1; + if (!path.isReferencedIdentifier()) { + return; + } + const binding = path.scope.getBinding(name); + if (binding && binding.scope !== path.scope.getProgramParent()) { + return; } + paths.push(path); }, }); - return references; + return paths; +}; + +export const countReferences = (program: t.Program, name: string): number => + referencePaths(program, name).length; + +export const topLevelReaders = (program: t.Program, name: string): t.Statement[] => { + const readers = new Set( + referencePaths(program, name).flatMap((path): t.Node[] => { + const statement = path.find((parent) => parent.parentPath?.isProgram() ?? false); + return statement ? [statement.node] : []; + }) + ); + return program.body.filter((node) => readers.has(node)); }; /** - * Strips the top-level `callee` calls and the imports that only fed them. Returns why the file was - * left alone, or `null` once removed. + * Strips the top-level `callee` calls and the imports that only fed them. `keptArguments` lists the + * identifier arguments whose imports survive because other code still reads them. * * Real previews wrap the call in a helper or pre-process its argument first. Rewriting those safely - * is not worth the risk, so anything that is not a plain top-level call is reported instead, and - * imports survive while any other code still reads them. + * is not worth the risk, so anything that is not a plain top-level call is reported instead. */ export const removeTopLevelCall = ( program: t.Program, { callee, calleeSources }: { callee: string; calleeSources: ReadonlySet } -): string | null => { +): RemoveTopLevelCallResult => { const callsToDrop = topLevelCalls(program, callee); if (callsToDrop.length === 0) { - return `${callee} is not called at the top level`; + return { removed: false, reason: `${callee} is not called at the top level` }; } const dropped = new Set(callsToDrop); const withoutCalls = t.program(program.body.filter((node) => !dropped.has(node))); if (countReferences(withoutCalls, callee) > 0) { - return `${callee} is still used elsewhere`; + return { removed: false, reason: `${callee} is still used elsewhere` }; } - const droppableImportNames = new Set( + const argumentNames = new Set( callsToDrop.flatMap((node): string[] => { const [argument] = (node.expression as t.CallExpression).arguments; - return t.isIdentifier(argument) && countReferences(withoutCalls, argument.name) === 0 - ? [argument.name] - : []; + return t.isIdentifier(argument) ? [argument.name] : []; }) ); + const keptArguments = [...argumentNames].filter( + (name) => countReferences(withoutCalls, name) > 0 + ); const remaining: t.Statement[] = []; for (const node of withoutCalls.body) { @@ -62,7 +85,8 @@ export const removeTopLevelCall = ( (specifier) => !( (calleeSources.has(node.source.value) && specifier.local.name === callee) || - droppableImportNames.has(specifier.local.name) + (argumentNames.has(specifier.local.name) && + !keptArguments.includes(specifier.local.name)) ) ); if (node.specifiers.length === 0) { @@ -73,5 +97,5 @@ export const removeTopLevelCall = ( } program.body = remaining; - return null; + return { removed: true, keptArguments }; };