Fix circular bundles, and never hoist the entry asset (#548)

devongovett · web-flow · commit dd26db34fb70 · 2018-01-11T16:30:10.000-08:00
Slightly simpler, more generic version of #489.
diff --git a/src/Bundler.js b/src/Bundler.js
@@ -389,7 +389,7 @@ class Bundler extends EventEmitter {
     this.buildQueue.delete(asset);
   }
 
-  createBundleTree(asset, dep, bundle) {
+  createBundleTree(asset, dep, bundle, parentBundles = new Set()) {
     if (dep) {
       asset.parentDeps.add(dep);
     }
@@ -405,7 +405,14 @@ class Bundler extends EventEmitter {
           this.moveAssetToBundle(asset, commonBundle);
           return;
         }
-      } else return;
+      } else {
+        return;
+      }
+
+      // Detect circular bundles
+      if (parentBundles.has(asset.parentBundle)) {
+        return;
+      }
     }
 
     // Create the root bundle if it doesn't exist
@@ -444,15 +451,22 @@ class Bundler extends EventEmitter {
     }
 
     asset.parentBundle = bundle;
+    parentBundles.add(bundle);
 
     for (let [dep, assetDep] of asset.depAssets) {
-      this.createBundleTree(assetDep, dep, bundle);
+      this.createBundleTree(assetDep, dep, bundle, parentBundles);
     }
 
+    parentBundles.delete(bundle);
     return bundle;
   }
 
   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) {
+      return;
+    }
+
     for (let bundle of Array.from(asset.bundles)) {
       bundle.removeAsset(asset);
       commonBundle.getSiblingBundle(bundle.type).addAsset(asset);
diff --git a/test/html.js b/test/html.js
@@ -299,4 +299,36 @@ describe('html', function() {
       ]
     });
   });
+
+  it('should support circular dependencies', async function() {
+    let b = await bundle(__dirname + '/integration/circular/index.html');
+
+    assertBundleTree(b, {
+      name: 'index.html',
+      assets: ['index.html'],
+      childBundles: [
+        {
+          type: 'html',
+          assets: ['about.html'],
+          childBundles: [
+            {
+              type: 'js',
+              assets: ['about.js', 'index.js'],
+              childBundles: []
+            },
+            {
+              type: 'html',
+              assets: ['test.html'],
+              childBundles: []
+            }
+          ]
+        },
+        {
+          type: 'js',
+          assets: ['about.js', 'index.js'],
+          childBundles: []
+        }
+      ]
+    });
+  });
 });
diff --git a/test/integration/circular/.eslintrc.json b/test/integration/circular/.eslintrc.json
@@ -0,0 +1,6 @@
+{
+  "extends": "../.eslintrc.json",
+  "parserOptions": {
+    "sourceType": "module"
+  }
+}
diff --git a/test/integration/circular/about.html b/test/integration/circular/about.html
@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+<html lang="en">
+<head>
+  <title>Document</title>
+  <script src="./about.js"></script>
+</head>
+<body>
+  On About
+
+  <a href="./index.html">Home</a>
+  <a href="./about.html">About</a>
+  <a href="./test.html">Test</a>
+
+</body>
+</html>
diff --git a/test/integration/circular/about.js b/test/integration/circular/about.js
@@ -0,0 +1,2 @@
+// eslint-disable-next-line no-unused-vars
+import index from './index';
diff --git a/test/integration/circular/index.html b/test/integration/circular/index.html
@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+<html lang="en">
+<head>
+  <title>Document</title>
+  <script src="./index.js"></script>
+</head>
+<body>
+  On Home
+
+  <a href="./index.html">Home</a>
+  <a href="./about.html">About</a>
+  <a href="./test.html">Test</a>
+
+</body>
+</html>
diff --git a/test/integration/circular/index.js b/test/integration/circular/index.js
@@ -0,0 +1,2 @@
+// eslint-disable-next-line no-unused-vars
+import about from './about';
diff --git a/test/integration/circular/test.html b/test/integration/circular/test.html
@@ -0,0 +1,14 @@
+<!DOCTYPE html>
+<html lang="en">
+<head>
+  <title>Document</title>
+</head>
+<body>
+  Test
+
+  <a href="./index.html">Home</a>
+  <a href="./about.html">About</a>
+  <a href="./test.html">Test</a>
+
+</body>
+</html>

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+// eslint-disable-next-line no-unused-vars`
	`2`	`+import index from './index';`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+// eslint-disable-next-line no-unused-vars`
	`2`	`+import about from './about';`