From d8db6bae08342764b3989187ad868c4717310481 Mon Sep 17 00:00:00 2001
From: Devon Govett <devongovett@gmail.com>
Date: Sun, 6 May 2018 16:36:33 -0700
Subject: [PATCH] Fix bundle hoisting when asset is already in the common
 bundle (#1310)

---
 src/Bundler.js                               | 13 +++++---
 test/integration/dynamic-hoist-dup/a.js      |  4 +++
 test/integration/dynamic-hoist-dup/common.js |  1 +
 test/integration/dynamic-hoist-dup/index.js  |  8 +++++
 test/javascript.js                           | 32 ++++++++++++++++++++
 5 files changed, 53 insertions(+), 5 deletions(-)
 create mode 100644 test/integration/dynamic-hoist-dup/a.js
 create mode 100644 test/integration/dynamic-hoist-dup/common.js
 create mode 100644 test/integration/dynamic-hoist-dup/index.js

diff --git a/src/Bundler.js b/src/Bundler.js
index 18cfc3f50c0..163cd6a1d48 100644
--- a/src/Bundler.js
+++ b/src/Bundler.js
@@ -559,10 +559,10 @@ class Bundler extends EventEmitter {
       // If the asset is already in a bundle, it is shared. Move it to the lowest common ancestor.
       if (asset.parentBundle !== bundle) {
         let commonBundle = bundle.findCommonAncestor(asset.parentBundle);
-        if (
-          asset.parentBundle !== commonBundle &&
-          asset.parentBundle.type === commonBundle.type
-        ) {
+
+        // If the common bundle's type matches the asset's, move the asset to the common bundle.
+        // Otherwise, proceed with adding the asset to the new bundle below.
+        if (asset.parentBundle.type === commonBundle.type) {
           this.moveAssetToBundle(asset, commonBundle);
           return;
         }
@@ -627,7 +627,10 @@ class Bundler extends EventEmitter {
 
   moveAssetToBundle(asset, commonBundle) {
     // Never move the entry asset of a bundle, as it was explicitly requested to be placed in a separate bundle.
-    if (asset.parentBundle.entryAsset === asset) {
+    if (
+      asset.parentBundle.entryAsset === asset ||
+      asset.parentBundle === commonBundle
+    ) {
       return;
     }
 
diff --git a/test/integration/dynamic-hoist-dup/a.js b/test/integration/dynamic-hoist-dup/a.js
new file mode 100644
index 00000000000..e0b45b97bd1
--- /dev/null
+++ b/test/integration/dynamic-hoist-dup/a.js
@@ -0,0 +1,4 @@
+var c = require('./common');
+
+exports.a = 1;
+exports.b = c;
diff --git a/test/integration/dynamic-hoist-dup/common.js b/test/integration/dynamic-hoist-dup/common.js
new file mode 100644
index 00000000000..4bbffde1044
--- /dev/null
+++ b/test/integration/dynamic-hoist-dup/common.js
@@ -0,0 +1 @@
+module.exports = 2;
diff --git a/test/integration/dynamic-hoist-dup/index.js b/test/integration/dynamic-hoist-dup/index.js
new file mode 100644
index 00000000000..a6e91424791
--- /dev/null
+++ b/test/integration/dynamic-hoist-dup/index.js
@@ -0,0 +1,8 @@
+var common = require('./common');
+var a = import('./a');
+
+module.exports = function () {
+  return a.then(function (a) {
+    return common + a.a + a.b;
+  });
+};
diff --git a/test/javascript.js b/test/javascript.js
index d061981604f..50af16704a6 100644
--- a/test/javascript.js
+++ b/test/javascript.js
@@ -281,6 +281,38 @@ describe('javascript', function() {
     assert.equal(await output(), 7);
   });
 
+  it('should not duplicate a module which is already in a parent bundle', async function() {
+    let b = await bundle(__dirname + '/integration/dynamic-hoist-dup/index.js');
+
+    assertBundleTree(b, {
+      name: 'index.js',
+      assets: [
+        'index.js',
+        'common.js',
+        'bundle-loader.js',
+        'bundle-url.js',
+        'js-loader.js'
+      ],
+      childBundles: [
+        {
+          assets: ['a.js'],
+          childBundles: [
+            {
+              type: 'map'
+            }
+          ]
+        },
+        {
+          type: 'map'
+        }
+      ]
+    });
+
+    let output = run(b);
+    assert.equal(typeof output, 'function');
+    assert.equal(await output(), 5);
+  });
+
   it('should support requiring JSON files', async function() {
     let b = await bundle(__dirname + '/integration/json/index.js');