fix(shaker): improve strategy for namespace imports and side effects (#…

…1243)

.changeset/thick-ads-beam.md

@@ -0,0 +1,8 @@
+---
+'@linaria/react': patch
+'@linaria/shaker': patch
+'@linaria/tags': patch
+'@linaria/utils': patch
+---
+
+Enhance @linaria/shaker strategy: better search in namespace imports, add support for side effect imports, fix file skipping.

packages/react/src/styled.ts

@@ -224,7 +224,7 @@ function styled(tag: any): any {
 
     // These properties will be read by the babel plugin for interpolation
     (Result as any).__linaria = {
-      className: options.class ?? mockedClass,
+      className: options.class || mockedClass,
       extends: tag,
     };
 

packages/shaker/src/plugins/__tests__/__snapshots__/shaker-plugin.test.ts.snap

@@ -2,12 +2,32 @@
 
 exports[`shaker should delete import 1`] = `"export const Alive = \\"\\";"`;
 
+exports[`shaker should handle __importDefault 1`] = `
+"var __importDefault = this && this.__importDefault || function (mod) {
+  return mod && mod.__esModule ? mod : {
+    default: mod
+  };
+};
+Object.defineProperty(exports, '__esModule', {
+  value: true
+});
+var Input_1 = require('./Input');
+Object.defineProperty(exports, 'Input', {
+  enumerable: true,
+  get: function () {
+    return __importDefault(Input_1).default;
+  }
+});"
+`;
+
 exports[`shaker should keep assigment even if export is marked for removing 1`] = `
 "let _a;
 exports.a = _a = {};
 exports.b = _a;"
 `;
 
+exports[`shaker should keep only side-effects 1`] = `"import 'regenerator-runtime/runtime.js';"`;
+
 exports[`shaker should keep referenced exports 1`] = `
 "var _foo;
 _foo = 10;
@@ -71,4 +91,9 @@ exports.a = _a = {};
 exports.b = _a;"
 `;
 
+exports[`shaker should shake if __linariaPreval required but not exported 1`] = `
+"import 'regenerator-runtime/runtime.js';
+export { Input } from \\"./Input\\";"
+`;
+
 exports[`shaker should throw out unused referenced exports 1`] = `"exports.defaultValue = 20;"`;

packages/shaker/src/plugins/__tests__/shaker-plugin.test.ts

@@ -335,4 +335,54 @@ describe('shaker', () => {
     expect(code).toMatchSnapshot();
     expect(metadata.imports.size).toBe(1);
   });
+
+  it('should keep only side-effects', () => {
+    const { code, metadata } = keep(['side-effect'])`
+      import 'regenerator-runtime/runtime.js';
+
+      export const a = 1;
+    `;
+
+    expect(code).toMatchSnapshot();
+    expect(metadata.imports.size).toBe(1);
+  });
+
+  it('should handle __importDefault', () => {
+    const { code, metadata } = keep(['Input'])`
+      var __importDefault =
+        (this && this.__importDefault) ||
+        function (mod) {
+          return mod && mod.__esModule ? mod : { default: mod };
+        };
+      Object.defineProperty(exports, '__esModule', { value: true });
+
+      var Input_1 = require('./Input');
+      Object.defineProperty(exports, 'Input', {
+        enumerable: true,
+        get: function () {
+          return __importDefault(Input_1).default;
+        },
+      });
+    `;
+
+    expect(code).toMatchSnapshot();
+    expect(metadata.imports.size).toBe(1);
+    expect(metadata.imports.get('./Input')).toEqual(['default']);
+  });
+
+  it('should shake if __linariaPreval required but not exported', () => {
+    const { code, metadata } = keep(['__linariaPreval', 'Input'])`
+      import 'regenerator-runtime/runtime.js';
+
+      export { Button } from "./Button";
+      export { Input } from "./Input";
+    `;
+
+    expect(code).toMatchSnapshot();
+    expect(metadata.imports.size).toBe(2);
+    expect([...metadata.imports.keys()]).toEqual([
+      'regenerator-runtime/runtime.js',
+      './Input',
+    ]);
+  });
 });

packages/shaker/src/plugins/shaker-plugin.ts

@@ -150,6 +150,7 @@ export default function shakerPlugin(
       const log = createCustomDebug('shaker', getFileIdx(this.filename));
 
       log('start', `${this.filename}, onlyExports: ${onlyExports.join(',')}`);
+      const onlyExportsSet = new Set(onlyExports);
 
       const collected = collectExportsAndImports(file.path);
       const sideEffectImports = collected.imports.filter(sideEffectImport);
@@ -171,6 +172,9 @@ export default function shakerPlugin(
         collected.exports
       );
 
+      const findExport = (name: string) =>
+        exports.find((i) => i.exported === name);
+
       collected.exports.forEach(({ local }) => {
         if (local.isAssignmentExpression()) {
           const left = local.get('left');
@@ -182,13 +186,16 @@ export default function shakerPlugin(
         }
       });
 
-      if (
-        onlyExports.length === 1 &&
-        onlyExports[0] === '__linariaPreval' &&
-        !exports.find((i) => i.exported === '__linariaPreval')
-      ) {
-        // Fast-lane: if only __linariaPreval is requested, and it's not exported,
-        // we can just shake out the whole file
+      const hasLinariaPreval = findExport('__linariaPreval') !== undefined;
+      const hasDefault = findExport('default') !== undefined;
+
+      // If __linariaPreval is not exported, we can remove it from onlyExports
+      if (onlyExportsSet.has('__linariaPreval') && !hasLinariaPreval) {
+        onlyExportsSet.delete('__linariaPreval');
+      }
+
+      if (onlyExportsSet.size === 0) {
+        // Fast-lane: if there are no exports to keep, we can just shake out the whole file
         this.imports = [];
         this.exports = [];
         this.reexports = [];
@@ -199,26 +206,31 @@ export default function shakerPlugin(
 
         return;
       }
+
+      const importedAsSideEffect = onlyExportsSet.has('side-effect');
+      onlyExportsSet.delete('side-effect');
+
       // Hackaround for packages which include a 'default' export without specifying __esModule; such packages cannot be
       // shaken as they will break interopRequireDefault babel helper
       // See example in shaker-plugin.test.ts
       // Real-world example was found in preact/compat npm package
       if (
-        onlyExports.includes('default') &&
-        exports.find(({ exported }) => exported === 'default') &&
+        onlyExportsSet.has('default') &&
+        hasDefault &&
         !collected.isEsModule
       ) {
         this.imports = collected.imports;
         this.exports = exports;
         this.reexports = collected.reexports;
         return;
       }
-      if (!onlyExports.includes('*')) {
+
+      if (!onlyExportsSet.has('*')) {
         const aliveExports = new Set<IExport | IReexport>();
         const importNames = collected.imports.map(({ imported }) => imported);
 
         exports.forEach((exp) => {
-          if (onlyExports.includes(exp.exported)) {
+          if (onlyExportsSet.has(exp.exported)) {
             aliveExports.add(exp);
           } else if (
             importNames.includes((exp.local.node as NodeWithName).name || '')
@@ -236,12 +248,12 @@ export default function shakerPlugin(
         });
 
         collected.reexports.forEach((exp) => {
-          if (onlyExports.includes(exp.exported)) {
+          if (onlyExportsSet.has(exp.exported)) {
             aliveExports.add(exp);
           }
         });
 
-        const isAllExportsFound = aliveExports.size === onlyExports.length;
+        const isAllExportsFound = aliveExports.size === onlyExportsSet.size;
         if (!isAllExportsFound && ifUnknownExport !== 'ignore') {
           if (ifUnknownExport === 'error') {
             throw new Error(
@@ -277,7 +289,7 @@ export default function shakerPlugin(
           .filter((exp) => !aliveExports.has(exp))
           .map((exp) => exp.local);
 
-        if (!keepSideEffects && sideEffectImports.length > 0) {
+        if (!keepSideEffects && !importedAsSideEffect) {
           // Remove all imports that don't import something explicitly and should not be kept
           sideEffectImports.forEach((i) => {
             if (!shouldKeepSideEffect(i.source)) {

packages/tags/src/TaggedTemplateProcessor.ts

@@ -9,7 +9,7 @@ import { validateParams } from './utils/validateParams';
 export default abstract class TaggedTemplateProcessor extends BaseProcessor {
   #template: (TemplateElement | ExpressionValue)[];
 
-  public constructor(params: Params, ...args: TailProcessorParams) {
+  protected constructor(params: Params, ...args: TailProcessorParams) {
     // Should have at least two params and the first one should be a callee.
     validateParams(
       params,

packages/utils/src/collectExportsAndImports.ts

@@ -36,7 +36,7 @@ import isRequire from './isRequire';
 import isTypedNode from './isTypedNode';
 
 export interface ISideEffectImport {
-  imported: null;
+  imported: 'side-effect';
   local: NodePath;
   source: string;
 }
@@ -70,11 +70,11 @@ export interface IState {
 
 export const sideEffectImport = (
   item: IImport | ISideEffectImport
-): item is ISideEffectImport => item.imported === null;
+): item is ISideEffectImport => item.imported === 'side-effect';
 
 export const explicitImport = (
   item: IImport | ISideEffectImport
-): item is IImport => item.imported !== null;
+): item is IImport => item.imported !== 'side-effect';
 
 function getValue({ node }: { node: Identifier | StringLiteral }): string {
   return node.type === 'Identifier' ? node.name : node.value;
@@ -130,7 +130,7 @@ function collectFromImportDeclaration(
   const specifiers = path.get('specifiers');
 
   if (specifiers.length === 0) {
-    state.imports.push({ imported: null, local: path, source });
+    state.imports.push({ imported: 'side-effect', local: path, source });
   }
 
   specifiers.forEach(<T extends SpecifierTypes>(specifier: NodePath<T>) => {
@@ -317,22 +317,27 @@ function collectFromDynamicImport(path: NodePath<Import>, state: IState): void {
   }
 }
 
-function getImportExportTypeByInteropFunction(
-  path: NodePath<CallExpression>
-): 'import:*' | 're-export:*' | 'default' | undefined {
+function getCalleeName(path: NodePath<CallExpression>): string | undefined {
   const callee = path.get('callee');
-  let name: string | undefined;
   if (callee.isIdentifier()) {
-    name = callee.node.name;
+    return callee.node.name;
   }
 
   if (callee.isMemberExpression()) {
     const property = callee.get('property');
     if (property.isIdentifier()) {
-      name = property.node.name;
+      return property.node.name;
     }
   }
 
+  return undefined;
+}
+
+function getImportExportTypeByInteropFunction(
+  path: NodePath<CallExpression>
+): 'import:*' | 're-export:*' | 'default' | undefined {
+  const name = getCalleeName(path);
+
   if (name === undefined) {
     return undefined;
   }
@@ -529,7 +534,7 @@ function collectFromRequire(path: NodePath<Identifier>, state: IState): void {
   if (container.isExpressionStatement()) {
     // Looks like standalone require
     state.imports.push({
-      imported: null,
+      imported: 'side-effect',
       local: container,
       source,
     });
@@ -765,7 +770,49 @@ function unfoldNamespaceImport(
       continue;
     }
 
-    if (parentPath?.isExportSpecifier()) {
+    if (
+      parentPath?.isCallExpression() &&
+      referencePath.listKey === 'arguments'
+    ) {
+      // The defined variable is used as a function argument. Let's try to figure out what is imported.
+      const importType = getImportExportTypeByInteropFunction(parentPath);
+
+      if (!importType) {
+        // Imported value is used as an unknown function argument,
+        // so we can't predict usage and import it as is.
+        result.push(importItem);
+        break;
+      }
+
+      if (importType === 'default') {
+        result.push({
+          ...importItem,
+          imported: 'default',
+          local: parentPath.get('id') as NodePath<Identifier>,
+        });
+
+        continue;
+      }
+
+      if (importType === 'import:*') {
+        result.push(importItem);
+        break;
+      }
+
+      warn(
+        'evaluator:collectExportsAndImports:unfoldNamespaceImports',
+        'Unknown import type',
+        importType
+      );
+
+      result.push(importItem);
+      continue;
+    }
+
+    if (
+      parentPath?.isExportSpecifier() ||
+      parentPath?.isExportDefaultDeclaration()
+    ) {
       // The whole namespace is re-exported
       result.push(importItem);
       break;