From 1ab05580396774f44b587d8ec3dc2d12ca13c2a0 Mon Sep 17 00:00:00 2001 From: Jasper De Moor Date: Tue, 3 Jul 2018 10:00:21 +0200 Subject: [PATCH] Fix worker bundle hoisting (#1257) --- src/Bundle.js | 16 ++++---- src/Bundler.js | 8 ++-- src/visitors/dependencies.js | 4 +- test/integration/workers/common.js | 4 ++ test/integration/workers/feature.js | 3 ++ test/integration/workers/index-alternative.js | 4 ++ test/integration/workers/index.js | 5 ++- test/integration/workers/worker-client.js | 11 +++++ test/integration/workers/worker.js | 6 ++- test/javascript.js | 41 ++++++++++++++++++- 10 files changed, 85 insertions(+), 17 deletions(-) create mode 100644 test/integration/workers/common.js create mode 100644 test/integration/workers/feature.js create mode 100644 test/integration/workers/index-alternative.js create mode 100644 test/integration/workers/worker-client.js diff --git a/src/Bundle.js b/src/Bundle.js index b8b742a7f74..0208c98c15f 100644 --- a/src/Bundle.js +++ b/src/Bundle.js @@ -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; @@ -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; @@ -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; } diff --git a/src/Bundler.js b/src/Bundler.js index 9fd063d97e1..791f7615b77 100644 --- a/src/Bundler.js +++ b/src/Bundler.js @@ -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); @@ -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) { @@ -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); } diff --git a/src/visitors/dependencies.js b/src/visitors/dependencies.js index 3e9e3cf0c9b..4f04a87baa9 100644 --- a/src/visitors/dependencies.js +++ b/src/visitors/dependencies.js @@ -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; } }, @@ -85,7 +85,7 @@ module.exports = { types.isStringLiteral(args[0]); if (isWebWorker) { - addURLDependency(asset, args[0]); + addURLDependency(asset, args[0], {isolated: true}); return; } } diff --git a/test/integration/workers/common.js b/test/integration/workers/common.js new file mode 100644 index 00000000000..787d00d7567 --- /dev/null +++ b/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; +}; diff --git a/test/integration/workers/feature.js b/test/integration/workers/feature.js new file mode 100644 index 00000000000..5add10c187a --- /dev/null +++ b/test/integration/workers/feature.js @@ -0,0 +1,3 @@ +const workerClient = require('./worker-client'); + +workerClient.startWorker(); diff --git a/test/integration/workers/index-alternative.js b/test/integration/workers/index-alternative.js new file mode 100644 index 00000000000..dfa6d9f99c4 --- /dev/null +++ b/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'); + diff --git a/test/integration/workers/index.js b/test/integration/workers/index.js index c168c379918..3e3e8fabe9c 100644 --- a/test/integration/workers/index.js +++ b/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'); diff --git a/test/integration/workers/worker-client.js b/test/integration/workers/worker-client.js new file mode 100644 index 00000000000..7505793ebce --- /dev/null +++ b/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); +}; + + + diff --git a/test/integration/workers/worker.js b/test/integration/workers/worker.js index d0965140fe7..dc7c4a135ac 100644 --- a/test/integration/workers/worker.js +++ b/test/integration/workers/worker.js @@ -1 +1,5 @@ -self.addEventListener('message', () => {}); +const commonText = require('./common').commonFunction('Worker'); + +self.addEventListener('message', () => { + self.postMessage(commonText); +}); diff --git a/test/javascript.js b/test/javascript.js index 4120c97d8f8..fdf19034966 100644 --- a/test/javascript.js +++ b/test/javascript.js @@ -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' @@ -172,7 +209,7 @@ describe('javascript', function() { ] }, { - assets: ['worker.js'], + assets: ['worker.js', 'common.js'], childBundles: [ { type: 'map'