fix(shaker): improved references detection (again fixes #1226) (#1274)

.changeset/nine-fans-walk.md

@@ -0,0 +1,6 @@
+---
+'@linaria/shaker': patch
+'@linaria/utils': patch
+---
+
+Improved compatibility with redux and some other libraries.

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

@@ -35,6 +35,16 @@ exports.defaultValue = Math.random() * _foo;
 exports.foo = _foo;"
 `;
 
+exports[`shaker should keep setBatch 1`] = `
+"function defaultNoopBatch(callback) {
+  callback();
+}
+var batch = defaultNoopBatch;
+export var setBatch = function setBatch(newBatch) {
+  return batch = newBatch;
+};"
+`;
+
 exports[`shaker should keep side-effects from modules 1`] = `
 "import 'regenerator-runtime/runtime.js';
 export const a = 1;"
@@ -45,6 +55,11 @@ exports[`shaker should process array patterns 1`] = `
 export { c };"
 `;
 
+exports[`shaker should process constant violations inside binding paths 1`] = `
+"export function c() {}
+;"
+`;
+
 exports[`shaker should process identifiers in void expressions as references 1`] = `
 "let _;
 function b(b) {

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

@@ -385,4 +385,38 @@ describe('shaker', () => {
       './Input',
     ]);
   });
+
+  it('should keep setBatch', () => {
+    // A real-world example from react-redux
+    const { code, metadata } = keep(['setBatch'])`
+      function defaultNoopBatch(callback) {
+        callback();
+      }
+
+      var batch = defaultNoopBatch;
+
+      export var setBatch = function setBatch(newBatch) {
+        return (batch = newBatch);
+      };
+
+      export var getBatch = function getBatch() {
+        return batch;
+      };
+    `;
+
+    expect(code).toMatchSnapshot();
+    expect(metadata.imports.size).toBe(0);
+  });
+
+  it('should process constant violations inside binding paths', () => {
+    // Function `a` should be removed because it's only used in removed function `b`
+    const { code, metadata } = keep(['c'])`
+      function a(flag) { return (a = function(flag) { flag ? 1 : 2 }) }
+      export function b() { return a(1) }
+      export function c() {};
+    `;
+
+    expect(code).toMatchSnapshot();
+    expect(metadata.imports.size).toBe(0);
+  });
 });

packages/utils/src/scopeHelpers.ts

@@ -61,36 +61,77 @@ export function reference(
   binding.referencePaths.push(referencePath ?? path);
 }
 
-function isReferenced(binding: Binding) {
-  if (!binding.referenced) {
+function isReferenced({ kind, referenced, referencePaths }: Binding) {
+  if (!referenced) {
     return false;
   }
 
   // If it's a param binding, we can't just remove it
   // because it brakes the function signature. Keep it alive for now.
-  if ((binding.kind as string) === 'param') {
+  if ((kind as string) === 'param') {
     return true;
   }
 
   // If all remaining references are in TS/Flow types, binding is unreferenced
-  return binding.referencePaths.some(
-    (i) => !i.find((ancestor) => ancestor.isTSType() || ancestor.isFlowType())
+  return (
+    referencePaths.length > 0 ||
+    referencePaths.every((i) =>
+      i.find((ancestor) => ancestor.isTSType() || ancestor.isFlowType())
+    )
   );
 }
 
+function isReferencedConstantViolation(path: NodePath, binding: Binding) {
+  if (path.find((p) => p === binding.path)) {
+    // function a(flag) { return (a = function(flag) { flag ? 1 : 2 }) }
+    // ^ Looks crazy, yeh? Welcome to the wonderful world of transpilers!
+    // `a = …` here isn't a reference.
+    return false;
+  }
+
+  if (!path.isReferenced()) {
+    return false;
+  }
+
+  if (
+    path.isAssignmentExpression() &&
+    path.parentPath.isExpressionStatement()
+  ) {
+    // A root assignment without a parent expression statement is not a reference
+    return false;
+  }
+
+  return true;
+}
+
 export function dereference(
   path: NodePath<Identifier | JSXIdentifier>
 ): Binding | null {
   const binding = getBinding(path);
   if (!binding) return null;
 
-  if (!binding.referencePaths.includes(path)) {
+  const isReference = binding.referencePaths.includes(path);
+  let referencesInConstantViolations = binding.constantViolations.filter((i) =>
+    isReferencedConstantViolation(i, binding)
+  );
+
+  const isConstantViolation = referencesInConstantViolations.includes(path);
+
+  if (!isReference && !isConstantViolation) {
     return null;
   }
 
-  binding.references -= 1;
-  binding.referencePaths = binding.referencePaths.filter((i) => i !== path);
-  binding.referenced = binding.referencePaths.length > 0;
+  if (isReference) {
+    binding.referencePaths = binding.referencePaths.filter((i) => i !== path);
+    binding.references -= 1;
+  } else {
+    referencesInConstantViolations = referencesInConstantViolations.filter(
+      (i) => i !== path
+    );
+  }
+
+  binding.referenced =
+    binding.referencePaths.length + referencesInConstantViolations.length > 0;
 
   return binding;
 }