ScopeHoistingPackager: Wrap assets when any ancestor is wrapped (#7883)

* Add test for wrapping assets, even if the first ancestor is not

* ScopeHoistingPackager: Handle different wrapped ancestries in wrapping dfs

packages/core/core/src/BundleGraph.js

@@ -860,10 +860,12 @@ export default class BundleGraph {
   traverseAssets<TContext>(
     bundle: Bundle,
     visit: GraphVisitor<Asset, TContext>,
+    startAsset?: Asset,
   ): ?TContext {
     return this.traverseBundle(
       bundle,
       mapVisitor(node => (node.type === 'asset' ? node.value : null), visit),
+      startAsset,
     );
   }
 
@@ -1057,8 +1059,9 @@ export default class BundleGraph {
   traverseBundle<TContext>(
     bundle: Bundle,
     visit: GraphVisitor<AssetNode | DependencyNode, TContext>,
+    startAsset?: Asset,
   ): ?TContext {
-    let entries = true;
+    let entries = !startAsset;
     let bundleNodeId = this._graph.getNodeIdByContentKey(bundle.id);
 
     // A modified DFS traversal which traverses entry assets in the same order
@@ -1085,7 +1088,9 @@ export default class BundleGraph {
 
         actions.skipChildren();
       }, visit),
-      startNodeId: bundleNodeId,
+      startNodeId: startAsset
+        ? this._graph.getNodeIdByContentKey(startAsset.id)
+        : bundleNodeId,
       getChildren: nodeId => {
         let children = this._graph
           .getNodeIdsConnectedFrom(nodeId)

packages/core/core/src/public/Bundle.js

@@ -186,10 +186,14 @@ export class Bundle implements IBundle {
     );
   }
 
-  traverseAssets<TContext>(visit: GraphVisitor<IAsset, TContext>): ?TContext {
+  traverseAssets<TContext>(
+    visit: GraphVisitor<IAsset, TContext>,
+    startAsset?: IAsset,
+  ): ?TContext {
     return this.#bundleGraph.traverseAssets(
       this.#bundle,
       mapVisitor(asset => assetFromValue(asset, this.#options), visit),
+      startAsset ? assetToAssetValue(startAsset) : undefined,
     );
   }
 }

packages/core/integration-tests/test/integration/scope-hoisting/es6/multiple-ancestors-wrap/index.js

@@ -0,0 +1,4 @@
+import value from './shouldBeWrapped';
+import otherValue from './wraps';
+
+output = [value, otherValue];

packages/core/integration-tests/test/integration/scope-hoisting/es6/multiple-ancestors-wrap/shouldBeWrapped.js

@@ -0,0 +1 @@
+export default 42;

packages/core/integration-tests/test/integration/scope-hoisting/es6/multiple-ancestors-wrap/wraps.js

@@ -0,0 +1,5 @@
+import value from './shouldBeWrapped';
+
+eval('void 0');
+
+export default value + 1;

packages/core/integration-tests/test/scope-hoisting.js

@@ -1003,6 +1003,27 @@ describe('scope hoisting', function () {
       assert.deepEqual(output, ['a', true]);
     });
 
+    it('wraps an asset if any of its ancestors is wrapped, even if one is not', async function () {
+      let b = await bundle(
+        path.join(
+          __dirname,
+          '/integration/scope-hoisting/es6/multiple-ancestors-wrap/index.js',
+        ),
+      );
+
+      let contents = await outputFS.readFile(
+        b.getBundles()[0].filePath,
+        'utf8',
+      );
+      assert.strictEqual(
+        contents.match(/parcelRequire.register\(/g).length,
+        2 /* once for parent asset, once for child wrapped asset */,
+      );
+
+      let output = await run(b);
+      assert.deepEqual(output, [42, 43]);
+    });
+
     it('supports importing from a wrapped asset with multiple bailouts', async function () {
       let b = await bundle(
         path.join(

packages/core/types/index.js

@@ -1247,7 +1247,10 @@ export interface Bundle {
   /** Returns whether the bundle includes the given dependency. */
   hasDependency(Dependency): boolean;
   /** Traverses the assets in the bundle. */
-  traverseAssets<TContext>(visit: GraphVisitor<Asset, TContext>): ?TContext;
+  traverseAssets<TContext>(
+    visit: GraphVisitor<Asset, TContext>,
+    startAsset?: Asset,
+  ): ?TContext;
   /** Traverses assets and dependencies in the bundle. */
   traverse<TContext>(
     visit: GraphVisitor<BundleTraversable, TContext>,

packages/packagers/js/src/ScopeHoistingPackager.js

@@ -247,7 +247,7 @@ export class ScopeHoistingPackager {
   async loadAssets(): Promise<Array<Asset>> {
     let queue = new PromiseQueue({maxConcurrent: 32});
     let wrapped = [];
-    this.bundle.traverseAssets((asset, shouldWrap) => {
+    this.bundle.traverseAssets(asset => {
       queue.add(async () => {
         let [code, map] = await Promise.all([
           asset.getCode(),
@@ -257,7 +257,6 @@ export class ScopeHoistingPackager {
       });
 
       if (
-        shouldWrap ||
         asset.meta.shouldWrap ||
         this.isAsyncBundle ||
         this.bundle.env.sourceType === 'script' ||
@@ -268,10 +267,25 @@ export class ScopeHoistingPackager {
       ) {
         this.wrappedAssets.add(asset.id);
         wrapped.push(asset);
-        return true;
       }
     });
 
+    for (let wrappedAssetRoot of [...wrapped]) {
+      this.bundle.traverseAssets((asset, _, actions) => {
+        if (asset === wrappedAssetRoot) {
+          return;
+        }
+
+        if (this.wrappedAssets.has(asset.id)) {
+          actions.skipChildren();
+          return;
+        }
+
+        this.wrappedAssets.add(asset.id);
+        wrapped.push(asset);
+      }, wrappedAssetRoot);
+    }
+
     this.assetOutputs = new Map(await queue.run());
     return wrapped;
   }