Skip to content

Commit

Permalink
Fix bundle hoisting when asset is already in the common bundle (#1310)
Browse files Browse the repository at this point in the history
  • Loading branch information
devongovett committed May 6, 2018
1 parent adeee42 commit d8db6ba
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 5 deletions.
13 changes: 8 additions & 5 deletions src/Bundler.js
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}

Expand Down
4 changes: 4 additions & 0 deletions test/integration/dynamic-hoist-dup/a.js
@@ -0,0 +1,4 @@
var c = require('./common');

exports.a = 1;
exports.b = c;
1 change: 1 addition & 0 deletions test/integration/dynamic-hoist-dup/common.js
@@ -0,0 +1 @@
module.exports = 2;
8 changes: 8 additions & 0 deletions 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;
});
};
32 changes: 32 additions & 0 deletions test/javascript.js
Expand Up @@ -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');

Expand Down

0 comments on commit d8db6ba

Please sign in to comment.