get tests to pass

parcel-bundler · Mar 18, 2021 · f28c04b · f28c04b
1 parent 6045068
commit f28c04b
Show file tree

Hide file tree

Showing 4 changed files with 77 additions and 16 deletions.
diff --git a/packages/bundlers/default/src/DefaultBundler.js b/packages/bundlers/default/src/DefaultBundler.js
@@ -439,7 +439,7 @@ function deduplicate(
           bundleGraph.isAssetReachableFromBundle(asset, bundle)
         ) {
           if (isSiblingDedupe) {
-            bundleGraph.removeAssetGraphFromBundle(asset, bundle, true);
+            bundleGraph.removeAssetGraphFromBundleIfReachable(asset, bundle);
           } else {
             bundleGraph.removeAssetGraphFromBundle(asset, bundle);
           }

diff --git a/packages/core/core/src/BundleGraph.js b/packages/core/core/src/BundleGraph.js
@@ -368,14 +368,9 @@ export default class BundleGraph {
     }
   }
 
-  removeAssetGraphFromBundle(
-    asset: Asset,
-    bundle: Bundle,
-    isSiblingDedupe?: boolean,
-  ) {
-    // If deduplicating between shared bundles, check for reachability
-    // before removing an asset. Else, indiscriminately remove all
-    // contains edges from the bundle to the nodes in the asset's subgraph.
+  removeAssetGraphFromBundle(asset: Asset, bundle: Bundle) {
+    // Indiscriminately remove all contains edges from the bundle to the nodes
+    // in the asset's subgraph.
     this._graph.traverse((node, context, actions) => {
       if (node.type === 'bundle_group') {
         actions.skipChildren();
@@ -387,14 +382,72 @@ export default class BundleGraph {
       }
 
       if (this._graph.hasEdge(bundle.id, node.id, 'contains')) {
-        if (
-          isSiblingDedupe &&
-          node.type === 'asset' &&
-          !this.isAssetReachableFromBundle(node.value, bundle)
-        ) {
-          return;
+        this._graph.removeEdge(
+          bundle.id,
+          node.id,
+          'contains',
+          // Removing this contains edge should not orphan the connected node. This
+          // is disabled for performance reasons as these edges are removed as part
+          // of a traversal, and checking for orphans becomes quite expensive in
+          // aggregate.
+          false /* removeOrphans */,
+        );
+
+        if (node.type === 'asset') {
+          bundle.stats.size -= asset.stats.size;
+        }
+      } else {
+        actions.skipChildren();
+        return;
+      }
+
+      if (node.type === 'asset' && this._graph.hasEdge(bundle.id, node.id)) {
+        // Remove the untyped edge from the bundle to the node (it's an entry)
+        this._graph.removeEdge(bundle.id, node.id);
+
+        let entryIndex = bundle.entryAssetIds.indexOf(node.value.id);
+        if (entryIndex >= 0) {
+          // Shared bundles have untyped edges to their asset graphs but don't
+          // have entry assets. For those that have entry asset ids, remove them.
+          bundle.entryAssetIds.splice(entryIndex, 1);
+        }
+      }
+
+      if (node.type === 'dependency') {
+        this.removeExternalDependency(bundle, node.value);
+        if (this._graph.hasEdge(bundle.id, node.id, 'references')) {
+          this._graph.removeEdge(bundle.id, node.id, 'references');
         }
+      }
+    }, nullthrows(this._graph.getNode(asset.id)));
+
+    // Remove bundle node if it no longer has any asset graphs
+    let bundleNode = nullthrows(this._graph.getNode(bundle.id));
+    if (this._graph.getNodesConnectedFrom(bundleNode).length === 0) {
+      this.removeBundle(bundle);
+    }
+
+    this._bundleContentHashes.delete(bundle.id);
+  }
+
+  removeAssetGraphFromBundleIfReachable(asset: Asset, bundle: Bundle) {
+    // Check for reachability as assets are removed to ensure they remain
+    // reachable from the bundle they are removed from.
+    this._graph.traverse((node, context, actions) => {
+      if (node.type === 'bundle_group') {
+        actions.skipChildren();
+        return;
+      }
+
+      if (node.type !== 'dependency' && node.type !== 'asset') {
+        return;
+      }
 
+      if (
+        this._graph.hasEdge(bundle.id, node.id, 'contains') &&
+        (node.type !== 'asset' ||
+          this.isAssetReachableFromBundle(node.value, bundle))
+      ) {
         this._graph.removeEdge(
           bundle.id,
           node.id,

diff --git a/packages/core/core/src/public/MutableBundleGraph.js b/packages/core/core/src/public/MutableBundleGraph.js
@@ -283,6 +283,13 @@ export default class MutableBundleGraph extends BundleGraph<IBundle>
     );
   }
 
+  removeAssetGraphFromBundleIfReachable(asset: IAsset, bundle: IBundle) {
+    this.#graph.removeAssetGraphFromBundleIfReachable(
+      assetToAssetValue(asset),
+      bundleToInternalBundle(bundle),
+    );
+  }
+
   traverseContents<TContext>(
     visit: GraphVisitor<BundlerBundleGraphTraversable, TContext>,
   ): ?TContext {

diff --git a/packages/core/types/index.js b/packages/core/types/index.js
@@ -982,7 +982,8 @@ export interface MutableBundleGraph extends BundleGraph<Bundle> {
   getParentBundlesOfBundleGroup(BundleGroup): Array<Bundle>;
   getTotalSize(Asset): number;
   /** Remove all "contains" edges from the bundle to the nodes in the asset's subgraph. */
-  removeAssetGraphFromBundle(Asset, Bundle, isSiblingDedupe?: boolean): void;
+  removeAssetGraphFromBundle(Asset, Bundle): void;
+  removeAssetGraphFromBundleIfReachable(Asset, Bundle): void;
   removeBundleGroup(bundleGroup: BundleGroup): void;
   /** Turns a dependency to a different bundle into a dependency to an asset inside <code>bundle</code>. */
   internalizeAsyncDependency(bundle: Bundle, dependency: Dependency): void;