diff --git a/src/packagers/JSPackager.js b/src/packagers/JSPackager.js index 15c058778be..0fe1c823eed 100644 --- a/src/packagers/JSPackager.js +++ b/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 += diff --git a/test/integration/js-different-contents/hello.js b/test/integration/js-different-contents/hello.js new file mode 100644 index 00000000000..1fdb8c5437e --- /dev/null +++ b/test/integration/js-different-contents/hello.js @@ -0,0 +1,2 @@ +const value = 'Hello' +export default value diff --git a/test/integration/js-different-contents/index.js b/test/integration/js-different-contents/index.js new file mode 100644 index 00000000000..9ed6b5eb460 --- /dev/null +++ b/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}!` diff --git a/test/integration/js-different-contents/world.js b/test/integration/js-different-contents/world.js new file mode 100644 index 00000000000..0b38c7ac009 --- /dev/null +++ b/test/integration/js-different-contents/world.js @@ -0,0 +1,2 @@ +const value = 'World' +export default value diff --git a/test/integration/js-same-contents-different-dependencies/hello/index.js b/test/integration/js-same-contents-different-dependencies/hello/index.js new file mode 100644 index 00000000000..afd0ec3785f --- /dev/null +++ b/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 diff --git a/test/integration/js-same-contents-different-dependencies/hello/value.js b/test/integration/js-same-contents-different-dependencies/hello/value.js new file mode 100644 index 00000000000..1fdb8c5437e --- /dev/null +++ b/test/integration/js-same-contents-different-dependencies/hello/value.js @@ -0,0 +1,2 @@ +const value = 'Hello' +export default value diff --git a/test/integration/js-same-contents-different-dependencies/index.js b/test/integration/js-same-contents-different-dependencies/index.js new file mode 100644 index 00000000000..a8a31cda1ad --- /dev/null +++ b/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}!` diff --git a/test/integration/js-same-contents-different-dependencies/world/index.js b/test/integration/js-same-contents-different-dependencies/world/index.js new file mode 100644 index 00000000000..afd0ec3785f --- /dev/null +++ b/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 diff --git a/test/integration/js-same-contents-different-dependencies/world/value.js b/test/integration/js-same-contents-different-dependencies/world/value.js new file mode 100644 index 00000000000..0b38c7ac009 --- /dev/null +++ b/test/integration/js-same-contents-different-dependencies/world/value.js @@ -0,0 +1,2 @@ +const value = 'World' +export default value diff --git a/test/integration/js-same-contents-same-dependencies/hello1.js b/test/integration/js-same-contents-same-dependencies/hello1.js new file mode 100644 index 00000000000..1fdb8c5437e --- /dev/null +++ b/test/integration/js-same-contents-same-dependencies/hello1.js @@ -0,0 +1,2 @@ +const value = 'Hello' +export default value diff --git a/test/integration/js-same-contents-same-dependencies/hello2.js b/test/integration/js-same-contents-same-dependencies/hello2.js new file mode 100644 index 00000000000..1fdb8c5437e --- /dev/null +++ b/test/integration/js-same-contents-same-dependencies/hello2.js @@ -0,0 +1,2 @@ +const value = 'Hello' +export default value diff --git a/test/integration/js-same-contents-same-dependencies/index.js b/test/integration/js-same-contents-same-dependencies/index.js new file mode 100644 index 00000000000..e18f9d7b754 --- /dev/null +++ b/test/integration/js-same-contents-same-dependencies/index.js @@ -0,0 +1,4 @@ +import hello1 from './hello1' +import hello2 from './hello2' + +export default `${hello1} ${hello2}!` diff --git a/test/javascript.js b/test/javascript.js index a833efb2d55..4120c97d8f8 100644 --- a/test/javascript.js +++ b/test/javascript.js @@ -968,4 +968,55 @@ describe('javascript', function() { await run(b, {define: mockDefine, module: undefined}); assert.equal(test, 2); }); + + it('should not dedupe imports with different contents', async function() { + let b = await bundle( + __dirname + `/integration/js-different-contents/index.js`, + { + hmr: false // enable asset dedupe in JSPackager + } + ); + + let module = await run(b); + assert.equal(module.default, 'Hello World!'); + }); + + it('should not dedupe imports with same content but different absolute dependency paths', async function() { + let b = await bundle( + __dirname + + `/integration/js-same-contents-different-dependencies/index.js`, + { + hmr: false // enable asset dedupe in JSPackager + } + ); + + let module = await run(b); + assert.equal(module.default, 'Hello World!'); + }); + + it('should dedupe imports with same content and same dependency paths', async function() { + let b = await bundle( + __dirname + `/integration/js-same-contents-same-dependencies/index.js`, + { + hmr: false // enable asset dedupe in JSPackager + } + ); + 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'))); + assert( + dedupedAssets.includes(path.join(rootDir, 'hello1.js')) || + dedupedAssets.includes(path.join(rootDir, 'hello2.js')) + ); + assert( + !( + dedupedAssets.includes(path.join(rootDir, 'hello1.js')) && + dedupedAssets.includes(path.join(rootDir, 'hello2.js')) + ) + ); + + let module = await run(b); + assert.equal(module.default, 'Hello Hello!'); + }); });