From a176dedc3ec60e88f8899614377d3f1fabe54ef7 Mon Sep 17 00:00:00 2001
From: Fathy Boundjadj <fathy.boundjadj@gmail.com>
Date: Thu, 3 May 2018 08:23:40 +0200
Subject: [PATCH] Ignore dependencies in falsy branches (#1166)

---
 src/assets/JSAsset.js                         |  8 +++++
 src/visitors/dependencies.js                  | 35 ++++++++++++++++++-
 src/visitors/env.js                           | 28 +++++++++++++++
 src/visitors/globals.js                       | 25 -------------
 test/integration/falsy-dep/index.js           | 27 ++++++++++++++
 test/integration/falsy-dep/true-alternate.js  |  0
 test/integration/falsy-dep/true-consequent.js |  0
 test/javascript.js                            | 17 +++++++++
 8 files changed, 114 insertions(+), 26 deletions(-)
 create mode 100644 src/visitors/env.js
 create mode 100644 test/integration/falsy-dep/index.js
 create mode 100644 test/integration/falsy-dep/true-alternate.js
 create mode 100644 test/integration/falsy-dep/true-consequent.js

diff --git a/src/assets/JSAsset.js b/src/assets/JSAsset.js
index 7261c359f74..f4231b73c12 100644
--- a/src/assets/JSAsset.js
+++ b/src/assets/JSAsset.js
@@ -7,12 +7,14 @@ const Asset = require('../Asset');
 const babylon = require('babylon');
 const insertGlobals = require('../visitors/globals');
 const fsVisitor = require('../visitors/fs');
+const envVisitor = require('../visitors/env');
 const babel = require('../transforms/babel');
 const generate = require('babel-generator').default;
 const uglify = require('../transforms/uglify');
 const SourceMap = require('../SourceMap');
 
 const IMPORT_RE = /\b(?:import\b|export\b|require\s*\()/;
+const ENV_RE = /\b(?:process\.env)\b/;
 const GLOBAL_RE = /\b(?:process|__dirname|__filename|global|Buffer)\b/;
 const FS_RE = /\breadFileSync\b/;
 const SW_RE = /\bnavigator\s*\.\s*serviceWorker\s*\.\s*register\s*\(/;
@@ -93,6 +95,12 @@ class JSAsset extends Asset {
 
   async pretransform() {
     await babel(this);
+
+    // Inline environment variables
+    if (ENV_RE.test(this.contents)) {
+      await this.parseIfNeeded();
+      this.traverseFast(envVisitor);
+    }
   }
 
   async transform() {
diff --git a/src/visitors/dependencies.js b/src/visitors/dependencies.js
index e697db75343..3e9e3cf0c9b 100644
--- a/src/visitors/dependencies.js
+++ b/src/visitors/dependencies.js
@@ -1,5 +1,6 @@
 const types = require('babel-types');
 const template = require('babel-template');
+const traverse = require('babel-traverse').default;
 const urlJoin = require('../utils/urlJoin');
 const isURL = require('../utils/is-url');
 const matchesPattern = require('./matches-pattern');
@@ -38,7 +39,8 @@ module.exports = {
       callee.name === 'require' &&
       args.length === 1 &&
       types.isStringLiteral(args[0]) &&
-      !hasBinding(ancestors, 'require');
+      !hasBinding(ancestors, 'require') &&
+      !isInFalsyBranch(ancestors);
 
     if (isRequire) {
       let optional = ancestors.some(a => types.isTryStatement(a)) || undefined;
@@ -116,6 +118,37 @@ function hasBinding(node, name) {
   return false;
 }
 
+function isInFalsyBranch(ancestors) {
+  // Check if any ancestors are if statements
+  return ancestors.some((node, index) => {
+    if (types.isIfStatement(node)) {
+      let res = evaluateExpression(node.test);
+      if (res && res.confident) {
+        // If the test is truthy, exclude the dep if it is in the alternate branch.
+        // If the test if falsy, exclude the dep if it is in the consequent branch.
+        let child = ancestors[index + 1];
+        return res.value ? child === node.alternate : child === node.consequent;
+      }
+    }
+  });
+}
+
+function evaluateExpression(node) {
+  // Wrap the node in a standalone program so we can traverse it
+  node = types.file(types.program([types.expressionStatement(node)]));
+
+  // Find the first expression and evaluate it.
+  let res = null;
+  traverse(node, {
+    Expression(path) {
+      res = path.evaluate();
+      path.stop();
+    }
+  });
+
+  return res;
+}
+
 function addDependency(asset, node, opts = {}) {
   if (asset.options.target !== 'browser') {
     const isRelativeImport = /^[/~.]/.test(node.value);
diff --git a/src/visitors/env.js b/src/visitors/env.js
new file mode 100644
index 00000000000..c060a4a376e
--- /dev/null
+++ b/src/visitors/env.js
@@ -0,0 +1,28 @@
+const types = require('babel-types');
+const matchesPattern = require('./matches-pattern');
+
+module.exports = {
+  MemberExpression(node, asset) {
+    // Inline environment variables accessed on process.env
+    if (matchesPattern(node.object, 'process.env')) {
+      let key = types.toComputedKey(node);
+      if (types.isStringLiteral(key)) {
+        let val = types.valueToNode(process.env[key.value]);
+        morph(node, val);
+        asset.isAstDirty = true;
+        asset.cacheData.env[key.value] = process.env[key.value];
+      }
+    }
+  }
+};
+
+// replace object properties
+function morph(object, newProperties) {
+  for (let key in object) {
+    delete object[key];
+  }
+
+  for (let key in newProperties) {
+    object[key] = newProperties[key];
+  }
+}
diff --git a/src/visitors/globals.js b/src/visitors/globals.js
index 274de30aec1..2b2e5a4a3f8 100644
--- a/src/visitors/globals.js
+++ b/src/visitors/globals.js
@@ -1,6 +1,5 @@
 const Path = require('path');
 const types = require('babel-types');
-const matchesPattern = require('./matches-pattern');
 
 const VARS = {
   process: asset => {
@@ -18,19 +17,6 @@ const VARS = {
 };
 
 module.exports = {
-  MemberExpression(node, asset) {
-    // Inline environment variables accessed on process.env
-    if (matchesPattern(node.object, 'process.env')) {
-      let key = types.toComputedKey(node);
-      if (types.isStringLiteral(key)) {
-        let val = types.valueToNode(process.env[key.value]);
-        morph(node, val);
-        asset.isAstDirty = true;
-        asset.cacheData.env[key.value] = process.env[key.value];
-      }
-    }
-  },
-
   Identifier(node, asset, ancestors) {
     let parent = ancestors[ancestors.length - 2];
     if (
@@ -63,14 +49,3 @@ function inScope(ancestors) {
 
   return false;
 }
-
-// replace object properties
-function morph(object, newProperties) {
-  for (let key in object) {
-    delete object[key];
-  }
-
-  for (let key in newProperties) {
-    object[key] = newProperties[key];
-  }
-}
diff --git a/test/integration/falsy-dep/index.js b/test/integration/falsy-dep/index.js
new file mode 100644
index 00000000000..d42ce74b972
--- /dev/null
+++ b/test/integration/falsy-dep/index.js
@@ -0,0 +1,27 @@
+if (false) {
+  require('if-false-optional-dep');
+}
+
+if (false) {
+  globalStuff(() =>
+    require('if-false-optional-dep-deep')
+  );
+}
+
+if ('') {
+  require('if-falsy-optional-dep');
+}
+
+if (process.env.NODE_ENV === 'test') {
+  require('./true-consequent');
+} else {
+  require('./false-alternate');
+}
+
+if (process.env.NODE_ENV !== 'test') {
+  require('./false-consequent');
+} else {
+  require('./true-alternate');
+}
+
+module.exports = 2;
diff --git a/test/integration/falsy-dep/true-alternate.js b/test/integration/falsy-dep/true-alternate.js
new file mode 100644
index 00000000000..e69de29bb2d
diff --git a/test/integration/falsy-dep/true-consequent.js b/test/integration/falsy-dep/true-consequent.js
new file mode 100644
index 00000000000..e69de29bb2d
diff --git a/test/javascript.js b/test/javascript.js
index eef8f896840..6000d04ad16 100644
--- a/test/javascript.js
+++ b/test/javascript.js
@@ -783,6 +783,23 @@ describe('javascript', function() {
     assert.deepEqual(output, err);
   });
 
+  it('should support excluding dependencies in falsy branches', async function() {
+    let b = await bundle(__dirname + '/integration/falsy-dep/index.js');
+
+    assertBundleTree(b, {
+      name: 'index.js',
+      assets: ['index.js', 'true-alternate.js', 'true-consequent.js'],
+      childBundles: [
+        {
+          type: 'map'
+        }
+      ]
+    });
+
+    let output = run(b);
+    assert.equal(output, 2);
+  });
+
   it('should not autoinstall if resolve failed on installed module', async function() {
     let error;
     try {