Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fix worker bundle hoisting (#1257)
  • Loading branch information
DeMoorJasper authored and devongovett committed Jul 3, 2018
1 parent f699e81 commit 1ab0558
Show file tree
Hide file tree
Showing 10 changed files with 85 additions and 17 deletions.
16 changes: 9 additions & 7 deletions src/Bundle.js
Expand Up @@ -8,7 +8,7 @@ const crypto = require('crypto');
* the bundle, e.g. importing a CSS file from JS.
*/
class Bundle {
constructor(type, name, parent) {
constructor(type, name, parent, options = {}) {
this.type = type;
this.name = name;
this.parentBundle = parent;
Expand All @@ -20,13 +20,15 @@ class Bundle {
this.offsets = new Map();
this.totalSize = 0;
this.bundleTime = 0;
this.isolated = options.isolated;
}

static createWithAsset(asset, parentBundle) {
static createWithAsset(asset, parentBundle, options) {
let bundle = new Bundle(
asset.type,
Path.join(asset.options.outDir, asset.generateBundleName()),
parentBundle
parentBundle,
options
);

bundle.entryAsset = asset;
Expand Down Expand Up @@ -75,14 +77,14 @@ class Bundle {
return this.siblingBundlesMap.get(type);
}

createChildBundle(entryAsset) {
let bundle = Bundle.createWithAsset(entryAsset, this);
createChildBundle(entryAsset, options = {}) {
let bundle = Bundle.createWithAsset(entryAsset, this, options);
this.childBundles.add(bundle);
return bundle;
}

createSiblingBundle(entryAsset) {
let bundle = this.createChildBundle(entryAsset);
createSiblingBundle(entryAsset, options = {}) {
let bundle = this.createChildBundle(entryAsset, options);
this.siblingBundles.add(bundle);
return bundle;
}
Expand Down
8 changes: 5 additions & 3 deletions src/Bundler.js
Expand Up @@ -585,7 +585,7 @@ class Bundler extends EventEmitter {
asset.parentDeps.add(dep);
}

if (asset.parentBundle) {
if (asset.parentBundle && !bundle.isolated) {
// 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);
Expand Down Expand Up @@ -616,7 +616,7 @@ class Bundler extends EventEmitter {
}

// Create a new bundle for dynamic imports
bundle = bundle.createChildBundle(asset);
bundle = bundle.createChildBundle(asset, dep);
} else if (asset.type && !this.packagers.has(asset.type)) {
// If the asset is already the entry asset of a bundle, don't create a duplicate.
if (isEntryAsset) {
Expand Down Expand Up @@ -665,7 +665,9 @@ class Bundler extends EventEmitter {
}

for (let bundle of Array.from(asset.bundles)) {
bundle.removeAsset(asset);
if (!bundle.isolated) {
bundle.removeAsset(asset);
}
commonBundle.getSiblingBundle(bundle.type).addAsset(asset);
}

Expand Down
4 changes: 2 additions & 2 deletions src/visitors/dependencies.js
Expand Up @@ -70,7 +70,7 @@ module.exports = {
if (isRegisterServiceWorker) {
// Treat service workers as an entry point so filenames remain consistent across builds.
// https://developers.google.com/web/fundamentals/primers/service-workers/lifecycle#avoid_changing_the_url_of_your_service_worker_script
addURLDependency(asset, args[0], {entry: true});
addURLDependency(asset, args[0], {entry: true, isolated: true});
return;
}
},
Expand All @@ -85,7 +85,7 @@ module.exports = {
types.isStringLiteral(args[0]);

if (isWebWorker) {
addURLDependency(asset, args[0]);
addURLDependency(asset, args[0], {isolated: true});
return;
}
}
Expand Down
4 changes: 4 additions & 0 deletions test/integration/workers/common.js
@@ -0,0 +1,4 @@
// required by worker and index, must be bundled separately
exports.commonFunction = function (source) {
return 'commonText' + source;
};
3 changes: 3 additions & 0 deletions test/integration/workers/feature.js
@@ -0,0 +1,3 @@
const workerClient = require('./worker-client');

workerClient.startWorker();
4 changes: 4 additions & 0 deletions test/integration/workers/index-alternative.js
@@ -0,0 +1,4 @@
exports.startWorker = require('./worker-client').startWorker;
exports.commonFunction = require('./common').commonFunction;
exports.feature = require('./feature');

5 changes: 3 additions & 2 deletions test/integration/workers/index.js
@@ -1,3 +1,4 @@
navigator.serviceWorker.register('service-worker.js', { scope: './' });
exports.commonFunction = require('./common').commonFunction;
exports.startWorker = require('./worker-client').startWorker;
exports.feature = require('./feature');

new Worker('worker.js');
11 changes: 11 additions & 0 deletions test/integration/workers/worker-client.js
@@ -0,0 +1,11 @@
const commonText = require('./common').commonFunction('Index');

navigator.serviceWorker.register('service-worker.js', { scope: './' });

exports.startWorker = function() {
const worker = new Worker('worker.js');
worker.postMessage(commonText);
};



6 changes: 5 additions & 1 deletion test/integration/workers/worker.js
@@ -1 +1,5 @@
self.addEventListener('message', () => {});
const commonText = require('./common').commonFunction('Worker');

self.addEventListener('message', () => {
self.postMessage(commonText);
});
41 changes: 39 additions & 2 deletions test/javascript.js
Expand Up @@ -158,7 +158,44 @@ describe('javascript', function() {

await assertBundleTree(b, {
name: 'index.js',
assets: ['index.js'],
assets: ['index.js', 'common.js', 'worker-client.js', 'feature.js'],
childBundles: [
{
type: 'map'
},
{
assets: ['service-worker.js'],
childBundles: [
{
type: 'map'
}
]
},
{
assets: ['worker.js', 'common.js'],
childBundles: [
{
type: 'map'
}
]
}
]
});
});

it('should support bundling workers with different order', async function() {
let b = await bundle(
__dirname + '/integration/workers/index-alternative.js'
);

assertBundleTree(b, {
name: 'index-alternative.js',
assets: [
'index-alternative.js',
'common.js',
'worker-client.js',
'feature.js'
],
childBundles: [
{
type: 'map'
Expand All @@ -172,7 +209,7 @@ describe('javascript', function() {
]
},
{
assets: ['worker.js'],
assets: ['worker.js', 'common.js'],
childBundles: [
{
type: 'map'
Expand Down

0 comments on commit 1ab0558

Please sign in to comment.