fixes #933 : JSPackager deduplication now accounts for differences in absolute dependency paths (#1011)

Author: pcattori

src/packagers/JSPackager.js

@@ -3,6 +3,7 @@ const path = require('path');
 const Packager = require('./Packager');
 const urlJoin = require('../utils/urlJoin');
 const lineCounter = require('../utils/lineCounter');
+const objectHash = require('../utils/objectHash');
 
 const prelude = {
   source: fs
@@ -35,13 +36,14 @@ class JSPackager extends Packager {
   }
 
   async addAsset(asset) {
-    if (this.dedupe.has(asset.generated.js)) {
+    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(asset.generated.js, asset.id);
+      this.dedupe.set(key, asset.id);
     }
 
     let deps = {};
@@ -60,7 +62,7 @@ class JSPackager extends Packager {
         deps[dep.name] = bundles;
         this.bundleLoaders.add(mod.type);
       } else {
-        deps[dep.name] = this.dedupe.get(mod.generated.js) || mod.id;
+        deps[dep.name] = this.dedupe.get(this.dedupeKey(mod)) || mod.id;
 
         // If the dep isn't in this bundle, add it to the list of external modules to preload.
         // Only do this if this is the root JS bundle, otherwise they will have already been
@@ -93,6 +95,14 @@ class JSPackager extends Packager {
     return name;
   }
 
+  dedupeKey(asset) {
+    // cannot rely *only* on generated JS for deduplication because paths like
+    // `../` can cause 2 identical JS files to behave differently depending on
+    // where they are located on the filesystem
+    let deps = Array.from(asset.depAssets.values(), dep => dep.name).sort();
+    return objectHash([asset.generated.js, deps]);
+  }
+
   async writeModule(id, code, deps = {}, map) {
     let wrapped = this.first ? '' : ',';
     wrapped +=

test/integration/js-different-contents/hello.js

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

test/integration/js-different-contents/index.js

@@ -0,0 +1,4 @@
+import hello from './hello.js'
+import world from './world.js'
+
+export default `${hello} ${world}!`

test/integration/js-different-contents/world.js

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

test/integration/js-same-contents-different-dependencies/hello/index.js

@@ -0,0 +1,3 @@
+// hello/index.js and world/index.js are exactly the same content-wise, but in different locations
+import value from './value'
+export default value

test/integration/js-same-contents-different-dependencies/hello/value.js

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

test/integration/js-same-contents-different-dependencies/index.js

@@ -0,0 +1,4 @@
+import hello from './hello'
+import world from './world'
+
+export default `${hello} ${world}!`

test/integration/js-same-contents-different-dependencies/world/index.js

@@ -0,0 +1,3 @@
+// hello/index.js and world/index.js are exactly the same content-wise, but in different locations
+import value from './value'
+export default value

test/integration/js-same-contents-different-dependencies/world/value.js

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

test/integration/js-same-contents-same-dependencies/hello1.js

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