Fix worker bundle hoisting

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;
   }

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);
     }
 

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;
     }
   }

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;
+};

test/integration/workers/feature.js

@@ -0,0 +1,3 @@
+const workerClient = require('./worker-client');
+
+workerClient.startWorker();

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');
+

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');

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);
+};
+
+
+

test/integration/workers/worker.js

@@ -1 +1,5 @@
-self.addEventListener('message', () => {});
+const commonText = require('./common').commonFunction('Worker');
+
+self.addEventListener('message', () => {
+  self.postMessage(commonText);
+});

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'