diff --git a/.changeset/cuddly-laws-drum.md b/.changeset/cuddly-laws-drum.md new file mode 100644 index 000000000..4d501c7ac --- /dev/null +++ b/.changeset/cuddly-laws-drum.md @@ -0,0 +1,6 @@ +--- +'@linaria/shaker': patch +'@linaria/utils': patch +--- + +Fix an incorrect dead-code detection when a function has a parameter with the same name as the function itself. Fixes #1055 diff --git a/.changeset/poor-socks-check.md b/.changeset/poor-socks-check.md new file mode 100644 index 000000000..efca69315 --- /dev/null +++ b/.changeset/poor-socks-check.md @@ -0,0 +1,6 @@ +--- +'@linaria/shaker': patch +'@linaria/utils': patch +--- + +Fix rare use case when `void`-expression causes too aggressive tree-shaking. Fixes #1055. diff --git a/packages/babel/src/index.ts b/packages/babel/src/index.ts index 3c2c6ff8c..c8d7450ab 100644 --- a/packages/babel/src/index.ts +++ b/packages/babel/src/index.ts @@ -13,6 +13,7 @@ import loadLinariaOptions from './transform-stages/helpers/loadLinariaOptions'; export { slugify } from '@linaria/utils'; +export { default as preeval } from './plugins/preeval'; export * from './utils/collectTemplateDependencies'; export { default as collectTemplateDependencies } from './utils/collectTemplateDependencies'; export { default as withLinariaMetadata } from './utils/withLinariaMetadata'; diff --git a/packages/shaker/src/plugins/__tests__/__snapshots__/shaker-plugin.test.ts.snap b/packages/shaker/src/plugins/__tests__/__snapshots__/shaker-plugin.test.ts.snap index 5bf829dd9..e5a73e892 100644 --- a/packages/shaker/src/plugins/__tests__/__snapshots__/shaker-plugin.test.ts.snap +++ b/packages/shaker/src/plugins/__tests__/__snapshots__/shaker-plugin.test.ts.snap @@ -20,6 +20,16 @@ exports[`shaker should process array patterns 1`] = ` export { c };" `; +exports[`shaker should process identifiers in void expressions as references 1`] = ` +"let _; + +function b(b) { + void _; +} + +exports.b = b;" +`; + exports[`shaker should process object patterns 1`] = ` "const { b: b1 diff --git a/packages/shaker/src/plugins/__tests__/shaker-plugin.test.ts b/packages/shaker/src/plugins/__tests__/shaker-plugin.test.ts index 12d21bae6..fe02d2a9b 100644 --- a/packages/shaker/src/plugins/__tests__/shaker-plugin.test.ts +++ b/packages/shaker/src/plugins/__tests__/shaker-plugin.test.ts @@ -161,4 +161,22 @@ describe('shaker', () => { expect(code).toMatchSnapshot(); expect(metadata.imports.size).toBe(0); }); + + it('should process identifiers in void expressions as references', () => { + const { code, metadata } = keep(['b'])` + let _; + + const a = void _; + + function b(b) { + void _; + } + + exports.a = a; + exports.b = b; + `; + + expect(code).toMatchSnapshot(); + expect(metadata.imports.size).toBe(0); + }); }); diff --git a/packages/testkit/src/__snapshots__/preeval.test.ts.snap b/packages/testkit/src/__snapshots__/preeval.test.ts.snap new file mode 100644 index 000000000..b63117040 --- /dev/null +++ b/packages/testkit/src/__snapshots__/preeval.test.ts.snap @@ -0,0 +1,21 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`preeval should keep getGlobal but remove window-related code 1`] = ` +"function getGlobal() { + if (typeof globalThis !== \\"undefined\\") { + return globalThis; + } + + if (typeof window !== \\"undefined\\") {} + + if (typeof global !== \\"undefined\\") { + return global; + } + + if (typeof self !== \\"undefined\\") { + return self; + } + + return mockGlobal; +}" +`; diff --git a/packages/testkit/src/preeval.test.ts b/packages/testkit/src/preeval.test.ts new file mode 100644 index 000000000..d08c32583 --- /dev/null +++ b/packages/testkit/src/preeval.test.ts @@ -0,0 +1,62 @@ +import { join } from 'path'; + +import { transformSync } from '@babel/core'; +import dedent from 'dedent'; + +import { preeval } from '@linaria/babel-preset'; + +const run = (code: TemplateStringsArray) => { + const filename = join(__dirname, 'source.js'); + const formattedCode = dedent(code); + + const transformed = transformSync(formattedCode, { + babelrc: false, + configFile: false, + filename, + plugins: [ + [ + preeval, + { + evaluate: true, + }, + ], + ], + }); + + if (!transformed) { + throw new Error(`Something went wrong with ${filename}`); + } + + return { + code: transformed.code, + // metadata: transformed.metadata.__linariaShaker, + }; +}; + +describe('preeval', () => { + it('should keep getGlobal but remove window-related code', () => { + const { code } = run` + function getGlobal() { + if (typeof globalThis !== "undefined") { + return globalThis; + } + + if (typeof window !== "undefined") { + return window; + } + + if (typeof global !== "undefined") { + return global; + } + + if (typeof self !== "undefined") { + return self; + } + + return mockGlobal; + } + `; + + expect(code).toMatchSnapshot(); + }); +}); diff --git a/packages/utils/src/collectExportsAndImports.ts b/packages/utils/src/collectExportsAndImports.ts index b7674df7c..c952d20eb 100644 --- a/packages/utils/src/collectExportsAndImports.ts +++ b/packages/utils/src/collectExportsAndImports.ts @@ -27,6 +27,7 @@ import type { import { warn } from '@linaria/logger'; +import { getScope } from './getScope'; import isExports from './isExports'; import isNotNull from './isNotNull'; import isRequire from './isRequire'; @@ -637,7 +638,7 @@ function unfoldNamespaceImport( return [importItem]; } - const binding = local.scope.getBinding(local.node.name); + const binding = getScope(local).getBinding(local.node.name); if (!binding?.referenced) { // Imported namespace is not referenced and probably not used, // but it can have side effects, so we should keep it as is diff --git a/packages/utils/src/findIdentifiers.ts b/packages/utils/src/findIdentifiers.ts index 570b77952..0dca88151 100644 --- a/packages/utils/src/findIdentifiers.ts +++ b/packages/utils/src/findIdentifiers.ts @@ -1,12 +1,29 @@ import type { NodePath } from '@babel/traverse'; import type { Node, Identifier, JSXIdentifier } from '@babel/types'; +import { getScope } from './getScope'; + type FindType = 'binding' | 'both' | 'referenced'; +function isInVoid(path: NodePath): boolean { + return path.parentPath?.isUnaryExpression({ operator: 'void' }) ?? false; +} + +function isBindingIdentifier(path: NodePath): path is NodePath { + return path.isBindingIdentifier() && !isInVoid(path); +} + +function isReferencedIdentifier( + path: NodePath +): path is NodePath { + return path.isReferencedIdentifier() || isInVoid(path); +} + +// For some reasons, `isBindingIdentifier` returns true for identifiers inside `void` expressions. const checkers: Record boolean> = { - binding: (ex) => ex.isBindingIdentifier(), - both: (ex) => ex.isBindingIdentifier() || ex.isReferencedIdentifier(), - referenced: (ex) => ex.isReferencedIdentifier(), + binding: (ex) => isBindingIdentifier(ex), + both: (ex) => isBindingIdentifier(ex) || isReferencedIdentifier(ex), + referenced: (ex) => isReferencedIdentifier(ex), }; export function nonType(path: NodePath): boolean { @@ -38,7 +55,7 @@ export default function findIdentifiers( // TODO: Is there a better way to check that it's a local variable? - const binding = path.scope.getBinding(path.node.name); + const binding = getScope(path).getBinding(path.node.name); if (!binding) { return; } diff --git a/packages/utils/src/getScope.ts b/packages/utils/src/getScope.ts new file mode 100644 index 000000000..ec94e61d5 --- /dev/null +++ b/packages/utils/src/getScope.ts @@ -0,0 +1,9 @@ +import type { NodePath } from '@babel/traverse'; + +export function getScope(path: NodePath) { + // In some nodes (like FunctionDeclaration) `scope` for `id` returns + // local function scope instead of a scope where function is declared. + return path.key === 'id' && path.parent === path.scope.block + ? path.scope.parent + : path.scope; +} diff --git a/packages/utils/src/isExports.ts b/packages/utils/src/isExports.ts index 71d662ee6..4b70771e0 100644 --- a/packages/utils/src/isExports.ts +++ b/packages/utils/src/isExports.ts @@ -1,5 +1,7 @@ import type { NodePath } from '@babel/traverse'; +import { getScope } from './getScope'; + /** * Checks that specified Identifier is a global `exports` * @param id @@ -9,8 +11,9 @@ export default function isExports(id: NodePath | null | undefined) { return false; } + const scope = getScope(id); + return ( - id.scope.getBinding('exports') === undefined && - id.scope.hasGlobal('exports') + scope.getBinding('exports') === undefined && scope.hasGlobal('exports') ); } diff --git a/packages/utils/src/isRequire.ts b/packages/utils/src/isRequire.ts index c2a06c8d1..c5e60d871 100644 --- a/packages/utils/src/isRequire.ts +++ b/packages/utils/src/isRequire.ts @@ -1,5 +1,7 @@ import type { NodePath } from '@babel/traverse'; +import { getScope } from './getScope'; + /** * Checks that specified Identifier is a global `require` * @param id @@ -9,8 +11,9 @@ export default function isRequire(id: NodePath | null | undefined) { return false; } + const scope = getScope(id); + return ( - id.scope.getBinding('require') === undefined && - id.scope.hasGlobal('require') + scope.getBinding('require') === undefined && scope.hasGlobal('require') ); } diff --git a/packages/utils/src/isUnnecessaryReactCall.ts b/packages/utils/src/isUnnecessaryReactCall.ts index 450b9b883..771bb1708 100644 --- a/packages/utils/src/isUnnecessaryReactCall.ts +++ b/packages/utils/src/isUnnecessaryReactCall.ts @@ -3,6 +3,7 @@ import type { CallExpression } from '@babel/types'; import type { IImport, ISideEffectImport } from './collectExportsAndImports'; import collectExportsAndImports from './collectExportsAndImports'; +import { getScope } from './getScope'; function getCallee(p: NodePath) { const callee = p.get('callee'); @@ -66,7 +67,7 @@ function isClassicReactRuntime( if (reactImports.length === 0) return false; const callee = getCallee(p); if (callee.isIdentifier() && isHookOrCreateElement(callee.node.name)) { - const bindingPath = callee.scope.getBinding(callee.node.name)?.path; + const bindingPath = getScope(callee).getBinding(callee.node.name)?.path; return reactImports.some((i) => bindingPath?.isAncestor(i.local)); } @@ -89,7 +90,7 @@ function isClassicReactRuntime( return false; } - const bindingPath = object.scope.getBinding(object.node.name)?.path; + const bindingPath = getScope(object).getBinding(object.node.name)?.path; return bindingPath?.isAncestor(defaultImport.local) ?? false; } diff --git a/packages/utils/src/scopeHelpers.ts b/packages/utils/src/scopeHelpers.ts index e69f90f56..025e44fdc 100644 --- a/packages/utils/src/scopeHelpers.ts +++ b/packages/utils/src/scopeHelpers.ts @@ -5,11 +5,12 @@ import type { Binding, NodePath } from '@babel/traverse'; import type { Identifier, JSXIdentifier, Program } from '@babel/types'; import findIdentifiers, { nonType } from './findIdentifiers'; +import { getScope } from './getScope'; import isNotNull from './isNotNull'; import isRemoved from './isRemoved'; function getBinding(path: NodePath) { - const binding = path.scope.getBinding(path.node.name); + const binding = getScope(path).getBinding(path.node.name); if (!binding) { return undefined; } @@ -275,7 +276,7 @@ function removeUnreferenced(items: NodePath[]) { const referenced = new Set>(); items.forEach((item) => { if (!item.node || isRemoved(item)) return; - const binding = item.scope.getBinding(item.node.name); + const binding = getScope(item).getBinding(item.node.name); if (!binding) return; const hasReferences = binding.referencePaths.filter((i) => !isRemoved(i)).length > 0; @@ -306,7 +307,8 @@ function removeUnreferenced(items: NodePath[]) { function removeWithRelated(paths: NodePath[]) { if (paths.length === 0) return; - const rootPath = paths[0].scope.getProgramParent().path as NodePath; + const rootPath = getScope(paths[0]).getProgramParent() + .path as NodePath; if (!fixed.has(rootPath)) { // Some libraries don't care about bindings, references, and other staff @@ -328,7 +330,7 @@ function removeWithRelated(paths: NodePath[]) { ); declared.push( ...findIdentifiers([deletingPath], 'binding').map( - (i) => i.scope.getBinding(i.node.name)! + (i) => getScope(i).getBinding(i.node.name)! ) ); diff --git a/packages/utils/src/visitors/JSXElementsRemover.ts b/packages/utils/src/visitors/JSXElementsRemover.ts index 63fc2043c..240a7127f 100644 --- a/packages/utils/src/visitors/JSXElementsRemover.ts +++ b/packages/utils/src/visitors/JSXElementsRemover.ts @@ -6,6 +6,7 @@ import type { JSX, } from '@babel/types'; +import { getScope } from '../getScope'; import { mutate } from '../scopeHelpers'; function getFunctionName(path: NodePath): string | null { @@ -24,7 +25,7 @@ export default function JSXElementsRemover( // We can do even more // If that JSX is a result of a function, we can replace the function body. - const functionScope = path.scope.getFunctionParent(); + const functionScope = getScope(path).getFunctionParent(); const scopePath = functionScope?.path; if (scopePath?.isFunction()) { const emptyBody = t.blockStatement([t.returnStatement(nullLiteral)]);