fix(shaker): keep exports if there are dependent code (fixes #804) (#807

)

packages/babel/__tests__/evaluators/__snapshots__/shaker.test.ts.snap

@@ -65,6 +65,18 @@ color.green = '#0f0';
 exports.__linariaPreval = [color];"
 `;
 
+exports[`keeps reused exports 1`] = `
+"\\"use strict\\";
+
+var bar = function bar() {
+  return 'hello world';
+};
+
+exports.bar = bar;
+var foo = exports.bar();
+exports.__linariaPreval = [foo];"
+`;
+
 exports[`removes all 1`] = `
 "\\"use strict\\";
 

packages/babel/__tests__/evaluators/shaker.test.ts

@@ -227,3 +227,17 @@ it('shakes for-in statements', () => {
 
   expect(shaken).toMatchSnapshot();
 });
+
+it('keeps reused exports', () => {
+  const [shaken] = _shake()`
+    const bar = function() {
+      return 'hello world';
+    };
+    exports.bar = bar;
+
+    const foo = exports.bar();
+    exports.__linariaPreval = [foo];
+  `;
+
+  expect(shaken).toMatchSnapshot();
+});

packages/shaker/package.json

@@ -50,7 +50,8 @@
     "@linaria/babel-preset": "^3.0.0-beta.7",
     "@linaria/logger": "^3.0.0-beta.3",
     "@linaria/preeval": "^3.0.0-beta.8",
-    "babel-plugin-transform-react-remove-prop-types": "^0.4.24"
+    "babel-plugin-transform-react-remove-prop-types": "^0.4.24",
+    "ts-invariant": "^0.9.0"
   },
   "peerDependencies": {
     "@babel/core": ">=7"

packages/shaker/src/DepsGraph.ts

@@ -1,10 +1,9 @@
 import { types as t } from '@babel/core';
-import type { Identifier, Node } from '@babel/types';
 import ScopeManager, { PromisedNode, resolveNode } from './scope';
 
-type Action = (this: DepsGraph, a: Node, b: Node) => void;
+type Action = (this: DepsGraph, a: t.Node, b: t.Node) => void;
 
-function addEdge(this: DepsGraph, a: Node, b: Node) {
+function addEdge(this: DepsGraph, a: t.Node, b: t.Node) {
   if (this.dependencies.has(a) && this.dependencies.get(a)!.has(b)) {
     // edge has been already added∂ƒ
     return;
@@ -25,17 +24,17 @@ function addEdge(this: DepsGraph, a: Node, b: Node) {
 }
 
 export default class DepsGraph {
-  public readonly imports: Map<string, Identifier[]> = new Map();
-  public readonly importAliases: Map<Identifier, string> = new Map();
+  public readonly imports: Map<string, t.Identifier[]> = new Map();
+  public readonly importAliases: Map<t.Identifier, string> = new Map();
   public readonly importTypes: Map<string, 'wildcard' | 'default'> = new Map();
 
-  protected readonly edges: Array<[Node, Node]> = [];
-  protected readonly exports: Map<string, Node> = new Map();
-  protected readonly dependencies: Map<Node, Set<Node>> = new Map();
-  protected readonly dependents: Map<Node, Set<Node>> = new Map();
+  protected readonly edges: Array<[t.Node, t.Node]> = [];
+  protected readonly exports: Map<string, t.Node> = new Map();
+  protected readonly dependencies: Map<t.Node, Set<t.Node>> = new Map();
+  protected readonly dependents: Map<t.Node, Set<t.Node>> = new Map();
 
   private actionQueue: Array<
-    [Action, Node | PromisedNode, Node | PromisedNode]
+    [Action, t.Node | PromisedNode, t.Node | PromisedNode]
   > = [];
 
   private processQueue() {
@@ -54,24 +53,24 @@ export default class DepsGraph {
     this.actionQueue = [];
   }
 
-  private getAllReferences(id: string): Identifier[] {
+  private getAllReferences(id: string): (t.Identifier | t.MemberExpression)[] {
     const [, name] = id.split(':');
     const declaration = this.scope.getDeclaration(id)!;
-    const allReferences = [
+    const allReferences: (t.Identifier | t.MemberExpression)[] = [
       ...Array.from(this.dependencies.get(declaration) || []),
       ...Array.from(this.dependents.get(declaration) || []),
-    ].filter((i) => t.isIdentifier(i) && i.name === name) as Identifier[];
+    ].filter((i) => t.isIdentifier(i) && i.name === name) as t.Identifier[];
     allReferences.push(declaration);
     return allReferences;
   }
 
   constructor(protected scope: ScopeManager) {}
 
-  addEdge(dependent: Node | PromisedNode, dependency: Node | PromisedNode) {
+  addEdge(dependent: t.Node | PromisedNode, dependency: t.Node | PromisedNode) {
     this.actionQueue.push([addEdge, dependent, dependency]);
   }
 
-  addExport(name: string, node: Node) {
+  addExport(name: string, node: t.Node) {
     this.exports.set(name, node);
   }
 
@@ -115,15 +114,15 @@ export default class DepsGraph {
       .map(([a]) => a);
   }
 
-  getDependencies(nodes: Node[]) {
+  getDependencies(nodes: t.Node[]) {
     this.processQueue();
     return nodes.reduce(
       (acc, node) => acc.concat(Array.from(this.dependencies.get(node) || [])),
-      [] as Node[]
+      [] as t.Node[]
     );
   }
 
-  getLeafs(only: string[] | null): Array<Node | undefined> {
+  getLeafs(only: string[] | null): Array<t.Node | undefined> {
     this.processQueue();
     return only
       ? only.map((name) => this.exports.get(name))

packages/shaker/src/langs/core.ts

@@ -312,10 +312,33 @@ export const visitors: Visitors = {
   MemberExpression(this: GraphBuilderState, node: MemberExpression) {
     this.baseVisit(node);
 
+    if (
+      isIdentifier(node.object, 'exports') &&
+      this.scope.getDeclaration(node.object) ===
+        ScopeManager.globalExportsIdentifier
+    ) {
+      // We treat `exports.something` and `exports['something']` as identifiers in the global scope
+      this.graph.addEdge(node, node.object);
+      this.graph.addEdge(node, node.property);
+
+      const isLVal = peek(this.context) === 'lval';
+      if (isLVal) {
+        this.scope.declare(node, false);
+      } else {
+        const declaration = this.scope.addReference(node);
+        this.graph.addEdge(node, declaration);
+      }
+
+      return;
+    }
+
     if (t.isIdentifier(node.object) && t.isIdentifier(node.property)) {
       // It's simple `foo.bar` expression. Is it a usage of a required library?
       const declaration = this.scope.getDeclaration(node.object);
-      if (declaration && this.graph.importAliases.has(declaration)) {
+      if (
+        t.isIdentifier(declaration) &&
+        this.graph.importAliases.has(declaration)
+      ) {
         // It is. We can remember what exactly we use from it.
         const source = this.graph.importAliases.get(declaration)!;
         this.graph.imports.get(source)!.push(node.property);

packages/shaker/src/scope.ts

@@ -1,18 +1,18 @@
 import { types as t } from '@babel/core';
-import type { Identifier, Node } from '@babel/types';
+import invariant from 'ts-invariant';
 
-type Scope = Map<string, Set<Identifier>>;
+type Scope = Map<string, Set<t.Identifier | t.MemberExpression>>;
 
-export type ScopeId = number | 'global';
+export type ScopeId = number | 'global' | 'exports';
 export type DeclareHandler = (
-  identifier: Identifier,
-  from: Identifier | null
+  identifier: t.Identifier,
+  from: t.Identifier | null
 ) => void;
 
 const ResolvedNode = Symbol('ResolvedNode');
 const functionScopes = new WeakSet<Scope>();
 
-export class PromisedNode<T = Node> {
+export class PromisedNode<T = t.Node> {
   static is<TNode>(obj: any): obj is PromisedNode<TNode> {
     return obj && ResolvedNode in obj;
   }
@@ -24,13 +24,37 @@ export class PromisedNode<T = Node> {
   }
 }
 
-export const resolveNode = <T = Node>(
+export const resolveNode = <T = t.Node>(
   obj: T | PromisedNode<T> | undefined
 ): T | undefined => (PromisedNode.is<T>(obj) ? obj.identifier : obj);
 
-const scopeIds = new WeakMap<Scope, number | 'global'>();
-const getId = (scope: Scope, identifier: Identifier): string =>
-  `${scopeIds.get(scope)}:${identifier.name}`;
+const getExportName = (node: t.Node): string => {
+  invariant(
+    t.isMemberExpression(node),
+    `getExportName expects MemberExpression but received ${node.type}`
+  );
+
+  const { object, property } = node;
+  invariant(
+    t.isIdentifier(object) && object.name === 'exports',
+    `getExportName expects a member expression with 'exports'`
+  );
+  invariant(
+    t.isIdentifier(property) || t.isStringLiteral(property),
+    `getExportName supports only identifiers and literals as names of exported values`
+  );
+
+  const name = t.isIdentifier(property) ? property.name : property.value;
+  return `exports.${name}`;
+};
+
+const scopeIds = new WeakMap<Scope, ScopeId>();
+const getId = (scope: Scope, identifier: t.Identifier | string): string => {
+  const scopeId = scopeIds.get(scope);
+  return `${scopeId}:${
+    typeof identifier === 'string' ? identifier : identifier.name
+  }`;
+};
 
 export default class ScopeManager {
   public static globalExportsIdentifier = t.identifier('exports');
@@ -41,7 +65,7 @@ export default class ScopeManager {
   private readonly handlers: Map<ScopeId, Array<DeclareHandler>> = new Map();
   private readonly declarations: Map<
     string,
-    Identifier | PromisedNode<Identifier>
+    t.Identifier | t.MemberExpression | PromisedNode<t.Identifier>
   > = new Map();
 
   private get global(): Scope {
@@ -77,11 +101,25 @@ export default class ScopeManager {
   }
 
   declare(
-    identifier: Identifier,
+    identifierOrMemberExpression: t.Identifier | t.MemberExpression,
     isHoistable: boolean,
-    from: Identifier | null = null,
+    from: t.Identifier | null = null,
     stack = 0
   ): void {
+    if (t.isMemberExpression(identifierOrMemberExpression)) {
+      // declare receives MemberExpression only if it's `exports.something` expression
+      const memberExp = identifierOrMemberExpression;
+      const name = getExportName(memberExp);
+      if (!this.global.has(name)) {
+        this.global.set(name, new Set());
+        this.declarations.set(getId(this.global, name), memberExp);
+      }
+
+      this.global.get(name)!.add(memberExp);
+      return;
+    }
+
+    const identifier = identifierOrMemberExpression;
     const idName = identifier.name;
     const scope = this.stack
       .slice(stack)
@@ -91,7 +129,7 @@ export default class ScopeManager {
       // Let's use naïve implementation of hoisting
       const promise = this.declarations.get(
         getId(this.global, identifier)
-      )! as PromisedNode<Identifier>;
+      )! as PromisedNode<t.Identifier>;
       promise[ResolvedNode] = identifier;
       scope.set(
         idName,
@@ -107,20 +145,24 @@ export default class ScopeManager {
     handlers.forEach((handler) => handler(identifier, from));
   }
 
-  addReference(identifier: Identifier): Identifier | PromisedNode {
-    const name = identifier.name;
-    const scope = this.stack.find((s) => s.has(name)) || this.global;
-    const id = getId(scope, identifier);
+  addReference(
+    identifierOrMemberExpression: t.Identifier | t.MemberExpression
+  ): t.Identifier | t.MemberExpression | PromisedNode {
+    const name = t.isIdentifier(identifierOrMemberExpression)
+      ? identifierOrMemberExpression.name
+      : getExportName(identifierOrMemberExpression);
+    const scope = this.stack.find((s) => s.has(name)) ?? this.global;
+    const id = getId(scope, name);
     if (scope === this.global && !scope.has(name)) {
       scope.set(name, new Set());
-      this.declarations.set(getId(scope, identifier), new PromisedNode());
+      this.declarations.set(id, new PromisedNode());
     }
 
-    scope.get(name)!.add(identifier);
+    scope.get(name)!.add(identifierOrMemberExpression);
     return this.declarations.get(id)!;
   }
 
-  whereIsDeclared(identifier: Identifier): ScopeId | undefined {
+  whereIsDeclared(identifier: t.Identifier): ScopeId | undefined {
     const name = identifier.name;
     const scope = this.stack.find(
       (s) => s.has(name) && s.get(name)!.has(identifier)
@@ -137,18 +179,20 @@ export default class ScopeManager {
   }
 
   getDeclaration(
-    identifierOrName: Identifier | string
-  ): Identifier | undefined {
+    identifierOrMemberExpOrName: t.Identifier | t.MemberExpression | string
+  ): t.Identifier | t.MemberExpression | undefined {
     let name: string;
-    if (typeof identifierOrName === 'string') {
-      name = identifierOrName;
+    if (typeof identifierOrMemberExpOrName === 'string') {
+      name = identifierOrMemberExpOrName;
+    } else if (t.isMemberExpression(identifierOrMemberExpOrName)) {
+      name = getId(this.global, getExportName(identifierOrMemberExpOrName));
     } else {
-      const scopeId = this.whereIsDeclared(identifierOrName);
+      const scopeId = this.whereIsDeclared(identifierOrMemberExpOrName);
       if (scopeId === undefined) {
         return undefined;
       }
 
-      name = getId(this.map.get(scopeId)!, identifierOrName);
+      name = getId(this.map.get(scopeId)!, identifierOrMemberExpOrName);
     }
 
     return resolveNode(this.declarations.get(name));

yarn.lock

@@ -13720,6 +13720,13 @@ trough@^1.0.0:
   resolved "https://registry.yarnpkg.com/trough/-/trough-1.0.5.tgz#b8b639cefad7d0bb2abd37d433ff8293efa5f406"
   integrity sha512-rvuRbTarPXmMb79SmzEp8aqXNKcK+y0XaB298IXueQ8I2PsrATcPBCSPyK/dDNa2iWOhKlfNnOjdAOTBU/nkFA==
 
+ts-invariant@^0.9.0:
+  version "0.9.0"
+  resolved "https://registry.yarnpkg.com/ts-invariant/-/ts-invariant-0.9.0.tgz#4c60e9159a31742ab0103f13d7f63314fb5409c9"
+  integrity sha512-+JqhKqywk+ue5JjAC6eTWe57mOIxYXypMUkBDStkAzvnlfkDJ1KGyeMuNRMwOt6GXzHSC1UT9JecowpZDmgXqA==
+  dependencies:
+    tslib "^2.1.0"
+
 tsconfig-paths@^3.9.0:
   version "3.9.0"
   resolved "https://registry.yarnpkg.com/tsconfig-paths/-/tsconfig-paths-3.9.0.tgz#098547a6c4448807e8fcb8eae081064ee9a3c90b"
@@ -13735,6 +13742,11 @@ tslib@^1.8.0, tslib@^1.8.1, tslib@^1.9.0:
   resolved "https://registry.yarnpkg.com/tslib/-/tslib-1.14.1.tgz#cf2d38bdc34a134bcaf1091c41f6619e2f672d00"
   integrity sha512-Xni35NKzjgMrwevysHTCArtLDpPvye8zV/0E4EyYn43P7/7qvQwPh9BGkHewbMulVntbigmcT7rdX3BNo9wRJg==
 
+tslib@^2.1.0:
+  version "2.3.0"
+  resolved "https://registry.yarnpkg.com/tslib/-/tslib-2.3.0.tgz#803b8cdab3e12ba581a4ca41c8839bbb0dacb09e"
+  integrity sha512-N82ooyxVNm6h1riLCoyS9e3fuJ3AMG2zIZs2Gd1ATcSFjSA23Q0fzjjZeh0jbJvWVDZ0cJT8yaNNaaXHzueNjg==
+
 tslint@5.14.0:
   version "5.14.0"
   resolved "https://registry.yarnpkg.com/tslint/-/tslint-5.14.0.tgz#be62637135ac244fc9b37ed6ea5252c9eba1616e"