Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ScopeHoistingPackager: Wrap assets when any ancestor is wrapped #7883

Merged
merged 8 commits into from Apr 11, 2022
9 changes: 7 additions & 2 deletions packages/core/core/src/BundleGraph.js
Expand Up @@ -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,
);
}

Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down
6 changes: 5 additions & 1 deletion packages/core/core/src/public/Bundle.js
Expand Up @@ -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,
);
}
}
Expand Down
@@ -0,0 +1,4 @@
import value from './shouldBeWrapped';
import otherValue from './wraps';

output = [value, otherValue];
@@ -0,0 +1 @@
export default 42;
@@ -0,0 +1,5 @@
import value from './shouldBeWrapped';

eval('void 0');

export default value + 1;
21 changes: 21 additions & 0 deletions packages/core/integration-tests/test/scope-hoisting.js
Expand Up @@ -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(
Expand Down
5 changes: 4 additions & 1 deletion packages/core/types/index.js
Expand Up @@ -1243,7 +1243,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>,
Expand Down
20 changes: 17 additions & 3 deletions packages/packagers/js/src/ScopeHoistingPackager.js
Expand Up @@ -241,7 +241,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(),
Expand All @@ -251,7 +251,6 @@ export class ScopeHoistingPackager {
});

if (
shouldWrap ||
asset.meta.shouldWrap ||
this.isAsyncBundle ||
this.bundle.env.sourceType === 'script' ||
Expand All @@ -262,10 +261,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;
}
Expand Down