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

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

@@ -35,6 +35,18 @@ exports.defaultValue = Math.random() * _foo;
 exports.foo = _foo;"
 `;
 
+exports[`shaker should keep setBatch 1`] = `
+"// Default to a dummy \\"batch\\" implementation that just runs the callback
+function defaultNoopBatch(callback) {
+  callback();
+}
+var batch = defaultNoopBatch; // Allow injecting another batching function later
+
+export var setBatch = function setBatch(newBatch) {
+  return batch = newBatch;
+}; // Supply a getter just to skip dealing with ESM bindings"
+`;
+
 exports[`shaker should keep side-effects from modules 1`] = `
 "import 'regenerator-runtime/runtime.js';
 export const a = 1;"

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

@@ -385,4 +385,26 @@ describe('shaker', () => {
       './Input',
     ]);
   });
+
+  it('should keep setBatch', () => {
+    const { code, metadata } = keep(['setBatch'])`
+      // Default to a dummy "batch" implementation that just runs the callback
+      function defaultNoopBatch(callback) {
+        callback();
+      }
+
+      var batch = defaultNoopBatch; // Allow injecting another batching function later
+
+      export var setBatch = function setBatch(newBatch) {
+        return (batch = newBatch);
+      }; // Supply a getter just to skip dealing with ESM bindings
+
+      export var getBatch = function getBatch() {
+        return batch;
+      };
+    `;
+
+    expect(code).toMatchSnapshot();
+    expect(metadata.imports.size).toBe(0);
+  });
 });

packages/utils/src/scopeHelpers.ts

@@ -61,36 +61,70 @@ 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) {
+  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(
+    isReferencedConstantViolation
+  );
+
+  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;
 }