fix(shaker): an issue with assigned exports (fixes #1348) (#1357)

.changeset/five-queens-scream.md

@@ -0,0 +1,6 @@
+---
+'@linaria/shaker': patch
+'@linaria/utils': patch
+---
+
+Fix an issue with Shaker when CJS export is assigned to a const. Fixes #1348.

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

@@ -50,6 +50,21 @@ exports[`shaker should keep side-effects from modules 1`] = `
 export const a = 1;"
 `;
 
+exports[`shaker should not remove exports in declarators 1`] = `
+"var _cellFrozenClassname;
+var _cellFrozen;
+var _cell;
+exports.__esModule = true;
+exports.cellFrozenClassname = exports.cellFrozen = exports.cellClassname = exports.cell = void 0;
+const cell = _cell = "cj343x07-0-0-beta-39";
+_cellClassname = \`rdg-cell \${cell}\`;
+const cellFrozen = _cellFrozen = "csofj7r7-0-0-beta-39";
+const cellFrozenClassname = _cellFrozenClassname = \`rdg-cell-frozen \${cellFrozen}\`;
+exports.cell = _cell;
+exports.cellFrozen = _cellFrozen;
+exports.cellFrozenClassname = _cellFrozenClassname;"
+`;
+
 exports[`shaker should not remove referenced export 1`] = `
 "export default class Media {}
 const _c = Media;

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

@@ -25,10 +25,10 @@ const getPresets = (extension: Extension) => {
 
 const keep =
   (only: string[], extension: Extension = 'js') =>
-  (code: TemplateStringsArray) => {
+  (code: TemplateStringsArray, ...args: string[]) => {
     const presets = getPresets(extension);
     const filename = join(__dirname, `source.${extension}`);
-    const formattedCode = dedent(code);
+    const formattedCode = dedent(code, ...args);
 
     const transformed = transformSync(formattedCode, {
       babelrc: false,
@@ -447,4 +447,21 @@ describe('shaker', () => {
     expect(code).toMatchSnapshot();
     expect(metadata.imports.size).toBe(0);
   });
+
+  it('should not remove exports in declarators', () => {
+    const { code, metadata } = keep(['cell', 'cellFrozen'])`
+      exports.__esModule = true;
+      exports.cellFrozenClassname = exports.cellFrozen = exports.cellClassname = exports.cell = void 0;
+      const cell = exports.cell = "cj343x07-0-0-beta-39";
+      const cellClassname = exports.cellClassname = \`rdg-cell ${'${cell}'}\`;
+      const cellFrozen = exports.cellFrozen = "csofj7r7-0-0-beta-39";
+      const cellFrozenClassname = exports.cellFrozenClassname = \`rdg-cell-frozen ${'${cellFrozen}'}\`;
+      exports.__linariaPreval = {};
+
+      exports.classes = { cellClassname };
+    `;
+
+    expect(code).toMatchSnapshot();
+    expect(metadata.imports.size).toBe(0);
+  });
 });

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

@@ -82,7 +82,18 @@ function rearrangeExports(
   const rootScope = root.scope;
   exportRefs.forEach((refs, name) => {
     if (refs.length <= 1) {
-      return;
+      if (refs.length === 1) {
+        // Maybe exports is assigned to another variable?
+        const declarator = refs[0].findParent((p) =>
+          p.isVariableDeclarator()
+        ) as NodePath<VariableDeclarator> | undefined;
+
+        if (!declarator) {
+          return;
+        }
+      } else {
+        return;
+      }
     }
 
     const uid = rootScope.generateUid(name);
@@ -98,6 +109,10 @@ function rearrangeExports(
       const [replaced] = ref.replaceWith(t.identifier(uid));
       if (replaced.isBindingIdentifier()) {
         rootScope.registerConstantViolation(replaced);
+        if (replaced.parentPath?.parentPath?.isVariableDeclarator()) {
+          // This is `const foo = exports.foo = "value"` case
+          reference(replaced, replaced, true);
+        }
       } else {
         reference(replaced);
       }
@@ -160,7 +175,7 @@ export default function shakerPlugin(
         collected.exports
       );
 
-      Object.values(collected.exports).forEach((local) => {
+      Object.values(exports).forEach((local) => {
         if (local.isAssignmentExpression()) {
           const left = local.get('left');
           if (left.isIdentifier()) {
@@ -213,6 +228,9 @@ export default function shakerPlugin(
       }
 
       if (!onlyExportsSet.has('*')) {
+        // __esModule should be kept alive
+        onlyExportsSet.add('__esModule');
+
         const aliveExports = new Set<NodePath>();
         const importNames = collected.imports.map(({ imported }) => imported);
 
@@ -238,7 +256,22 @@ export default function shakerPlugin(
           }
         });
 
-        const isAllExportsFound = aliveExports.size === onlyExportsSet.size;
+        const exportToPath = new Map<string, NodePath>();
+        Object.entries(exports).forEach(([exported, local]) => {
+          exportToPath.set(exported, local);
+        });
+
+        collected.reexports.forEach((exp) => {
+          exportToPath.set(exp.exported, exp.local);
+        });
+
+        const notFoundExports = [...onlyExportsSet].filter(
+          (exp) =>
+            exp !== '__esModule' && !aliveExports.has(exportToPath.get(exp)!)
+        );
+        exportToPath.clear();
+
+        const isAllExportsFound = notFoundExports.length === 0;
         if (!isAllExportsFound && ifUnknownExport !== 'ignore') {
           if (ifUnknownExport === 'error') {
             throw new Error(
@@ -285,12 +318,17 @@ export default function shakerPlugin(
 
         const deleted = new Set<NodePath>();
 
-        const dereferenced: NodePath<Identifier>[] = [];
+        let dereferenced: NodePath<Identifier>[] = [];
         let changed = true;
         while (changed && deleted.size < forDeleting.length) {
           changed = false;
           // eslint-disable-next-line no-restricted-syntax
           for (const path of forDeleting) {
+            if (deleted.has(path)) {
+              // eslint-disable-next-line no-continue
+              continue;
+            }
+
             const binding = getBindingForExport(path);
             const action = findActionForNode(path);
             const parent = action?.[1];
@@ -317,14 +355,16 @@ export default function shakerPlugin(
               changed = true;
             }
           }
-        }
 
-        dereferenced.forEach((path) => {
-          // If path is still alive, we need to reference it back
-          if (!isRemoved(path)) {
-            reference(path);
-          }
-        });
+          dereferenced.forEach((path) => {
+            // If path is still alive, we need to reference it back
+            if (!isRemoved(path)) {
+              reference(path);
+            }
+          });
+
+          dereferenced = [];
+        }
       }
 
       this.imports = withoutRemoved(collected.imports);

packages/utils/src/scopeHelpers.ts

@@ -668,6 +668,22 @@ function mutate<T extends NodePath>(path: T, fn: (p: T) => NodePath[] | void) {
         'name' in declared[0] &&
         declared[0].name === binding.identifier.name
       ) {
+        const init = assignment.get('init');
+        if (!Array.isArray(init) && init?.isAssignmentExpression()) {
+          // `const a = b = 1` → `b = 1`
+          assignment.parentPath?.replaceWith({
+            type: 'ExpressionStatement',
+            expression: init.node,
+          });
+
+          const left = init.get('left');
+          if (left.isIdentifier()) {
+            // If it was forcefully referenced in the shaker
+            dereference(left);
+          }
+
+          return;
+        }
         // Only one identifier is declared, so we can remove the whole declaration
         forDeleting.push(assignment);
         return;

turbo.json

@@ -28,7 +28,8 @@
       "dependsOn": ["^build"]
     },
     "test:dts": {
-      "dependsOn": ["build:declarations", "^build:declarations"]
+      "dependsOn": ["build:declarations", "^build:declarations"],
+      "cache": false
     },
     "typecheck": {
       "dependsOn": ["build:declarations", "^build:declarations"]