Skip to content

Commit

Permalink
Don't dedupe assets that are depended on in more than one bundle (#2122)
Browse files Browse the repository at this point in the history
  • Loading branch information
devongovett committed Oct 11, 2018
1 parent a599d59 commit 6eb84c7
Show file tree
Hide file tree
Showing 7 changed files with 59 additions and 14 deletions.
23 changes: 16 additions & 7 deletions packages/core/parcel-bundler/src/packagers/JSPackager.js
Expand Up @@ -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 = {};
Expand Down
@@ -0,0 +1,3 @@
import hello2 from './hello2'

export default `${hello2}`
@@ -0,0 +1,2 @@
const value = 'Hello'
export default value
@@ -0,0 +1,2 @@
const value = 'Hello'
export default value
@@ -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}`;
});
}
@@ -0,0 +1,3 @@
{
"browserslist": ["last 1 Chrome version"]
}
32 changes: 25 additions & 7 deletions packages/core/parcel-bundler/test/javascript.js
Expand Up @@ -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'),
Expand Down

0 comments on commit 6eb84c7

Please sign in to comment.