Don't dedupe assets that are depended on in more than one bundle (#2122)

src/packagers/JSPackager.js

@@ -31,14 +31,23 @@ class JSPackager extends Packager {
   }
 
   async addAsset(asset) {
-    let key = this.dedupeKey(asset);
-    if (this.dedupe.has(key)) {
-      return;
-    }
+    // If this module is referenced by another JS bundle, it needs to be exposed externally.
+    // In that case, don't dedupe the asset as it would affect the module ids that are referenced by other bundles.
+    let isExposed = !Array.from(asset.parentDeps).every(dep => {
+      let depAsset = this.bundler.loadedAssets.get(dep.parent);
+      return this.bundle.assets.has(depAsset) || depAsset.type !== 'js';
+    });
+
+    if (!isExposed) {
+      let key = this.dedupeKey(asset);
+      if (this.dedupe.has(key)) {
+        return;
+      }
 
-    // Don't dedupe when HMR is turned on since it messes with the asset ids
-    if (!this.options.hmr) {
-      this.dedupe.set(key, asset.id);
+      // Don't dedupe when HMR is turned on since it messes with the asset ids
+      if (!this.options.hmr) {
+        this.dedupe.set(key, asset.id);
+      }
     }
 
     let deps = {};

test/integration/js-dedup-hoist/a.js

@@ -0,0 +1,3 @@
+import hello2 from './hello2'
+
+export default `${hello2}`

test/integration/js-dedup-hoist/hello1.js

@@ -0,0 +1,2 @@
+const value = 'Hello'
+export default value

test/integration/js-dedup-hoist/hello2.js

@@ -0,0 +1,2 @@
+const value = 'Hello'
+export default value

test/integration/js-dedup-hoist/index.js

@@ -0,0 +1,8 @@
+import hello1 from './hello1'
+import hello2 from './hello2'
+
+export default function () {
+  return import('./a').then(function (a) {
+    return `${hello1} ${hello2}! ${a.default}`;
+  });
+}

test/integration/js-dedup-hoist/package.json

@@ -0,0 +1,3 @@
+{
+  "browserslist": ["last 1 Chrome version"]
+}

test/javascript.js

@@ -1520,24 +1520,42 @@ describe('javascript', function() {
       }
     );
     const {rootDir} = b.entryAsset.options;
-    const dedupedAssets = Array.from(b.offsets.keys()).map(asset => asset.name);
-    assert.equal(dedupedAssets.length, 2);
-    assert(dedupedAssets.includes(path.join(rootDir, 'index.js')));
+    const writtenAssets = Array.from(b.offsets.keys()).map(asset => asset.name);
+    assert.equal(writtenAssets.length, 2);
+    assert(writtenAssets.includes(path.join(rootDir, 'index.js')));
     assert(
-      dedupedAssets.includes(path.join(rootDir, 'hello1.js')) ||
-        dedupedAssets.includes(path.join(rootDir, 'hello2.js'))
+      writtenAssets.includes(path.join(rootDir, 'hello1.js')) ||
+        writtenAssets.includes(path.join(rootDir, 'hello2.js'))
     );
     assert(
       !(
-        dedupedAssets.includes(path.join(rootDir, 'hello1.js')) &&
-        dedupedAssets.includes(path.join(rootDir, 'hello2.js'))
+        writtenAssets.includes(path.join(rootDir, 'hello1.js')) &&
+        writtenAssets.includes(path.join(rootDir, 'hello2.js'))
       )
     );
 
     let module = await run(b);
     assert.equal(module.default, 'Hello Hello!');
   });
 
+  it('should not dedupe assets that exist in more than one bundle', async function() {
+    let b = await bundle(
+      path.join(__dirname, `/integration/js-dedup-hoist/index.js`),
+      {
+        hmr: false // enable asset dedupe in JSPackager
+      }
+    );
+    const {rootDir} = b.entryAsset.options;
+    const writtenAssets = Array.from(b.offsets.keys()).map(asset => asset.name);
+    assert(
+      writtenAssets.includes(path.join(rootDir, 'hello1.js')) &&
+        writtenAssets.includes(path.join(rootDir, 'hello2.js'))
+    );
+
+    let module = await run(b);
+    assert.equal(await module.default(), 'Hello Hello! Hello');
+  });
+
   it('should support importing HTML from JS async', async function() {
     let b = await bundle(
       path.join(__dirname, '/integration/import-html-async/index.js'),