Skip to content

Commit cb9de9d

Browse files
devongovettdevongovett
authored
Code split re-exports when using non-scope hoisting packager (#10207)
1 parent dadcead commit cb9de9d

File tree

8 files changed

+201
-102
lines changed

8 files changed

+201
-102
lines changed

‎packages/core/core/src/BundleGraph.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,6 @@ export default class BundleGraph {
199199
if (
200200
node.type === 'dependency' &&
201201
node.value.symbols != null &&
202-
node.value.env.shouldScopeHoist &&
203202
// Disable in dev mode because this feature is at odds with safeToIncrementallyBundle
204203
isProduction
205204
) {
@@ -290,7 +289,7 @@ export default class BundleGraph {
290289
for (let [as, from] of target) {
291290
let existing = nodeValueSymbols.get(as);
292291
if (existing) {
293-
symbols.set(from, existing);
292+
symbols.set(from, {...existing, meta: {rewritten: as}});
294293
} else {
295294
invariant(isReexportAll);
296295
if (as === from) {

‎packages/core/integration-tests/test/javascript.js

Lines changed: 27 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -4867,11 +4867,11 @@ describe('javascript', function () {
48674867
},
48684868
mode: 'production',
48694869
};
4870-
let usesSymbolPropagation = shouldScopeHoist;
4870+
let usesSymbolPropagation = true;
48714871
describe(`sideEffects: false with${
48724872
shouldScopeHoist ? '' : 'out'
48734873
} scope-hoisting`, function () {
4874-
if (usesSymbolPropagation) {
4874+
if (shouldScopeHoist) {
48754875
it('supports excluding unused CSS imports', async function () {
48764876
let b = await bundle(
48774877
path.join(
@@ -4965,9 +4965,9 @@ describe('javascript', function () {
49654965
assertBundles(b, [
49664966
{
49674967
type: 'js',
4968-
assets: usesSymbolPropagation
4968+
assets: shouldScopeHoist
49694969
? ['a.js', 'message1.js']
4970-
: ['a.js', 'esmodule-helpers.js', 'index.js', 'message1.js'],
4970+
: ['a.js', 'esmodule-helpers.js', 'message1.js'],
49714971
},
49724972
]);
49734973

@@ -4987,10 +4987,7 @@ describe('javascript', function () {
49874987
{require: false},
49884988
);
49894989

4990-
assert.deepEqual(
4991-
calls,
4992-
shouldScopeHoist ? ['message1'] : ['message1', 'index'],
4993-
);
4990+
assert.deepEqual(calls, ['message1']);
49944991
assert.deepEqual(res.output, 'Message 1');
49954992
});
49964993

@@ -5035,10 +5032,7 @@ describe('javascript', function () {
50355032
{require: false},
50365033
);
50375034

5038-
assert.deepEqual(
5039-
calls,
5040-
shouldScopeHoist ? ['message2'] : ['message2', 'index'],
5041-
);
5035+
assert.deepEqual(calls, ['message2']);
50425036
assert.deepEqual(res.output, 'Message 2');
50435037
});
50445038

@@ -5054,9 +5048,9 @@ describe('javascript', function () {
50545048
assertBundles(b, [
50555049
{
50565050
type: 'js',
5057-
assets: usesSymbolPropagation
5051+
assets: shouldScopeHoist
50585052
? ['c.js', 'message3.js']
5059-
: ['c.js', 'esmodule-helpers.js', 'index.js', 'message3.js'],
5053+
: ['c.js', 'esmodule-helpers.js', 'message3.js'],
50605054
},
50615055
]);
50625056

@@ -5075,10 +5069,7 @@ describe('javascript', function () {
50755069
{require: false},
50765070
);
50775071

5078-
assert.deepEqual(
5079-
calls,
5080-
shouldScopeHoist ? ['message3'] : ['message3', 'index'],
5081-
);
5072+
assert.deepEqual(calls, ['message3']);
50825073
assert.deepEqual(res.output, {default: 'Message 3'});
50835074
});
50845075

@@ -5136,15 +5127,11 @@ describe('javascript', function () {
51365127
{require: false},
51375128
);
51385129

5139-
if (shouldScopeHoist) {
5140-
try {
5141-
assert.deepEqual(calls, ['key', 'foo', 'index']);
5142-
} catch (e) {
5143-
// A different dependency order, but this is deemed acceptable as it's sideeffect free
5144-
assert.deepEqual(calls, ['foo', 'key', 'index']);
5145-
}
5146-
} else {
5147-
assert.deepEqual(calls, ['key', 'foo', 'types', 'index']);
5130+
try {
5131+
assert.deepEqual(calls, ['key', 'foo', 'index']);
5132+
} catch (e) {
5133+
// A different dependency order, but this is deemed acceptable as it's sideeffect free
5134+
assert.deepEqual(calls, ['foo', 'key', 'index']);
51485135
}
51495136

51505137
assert.deepEqual(res.output, ['key', 'foo']);
@@ -5569,10 +5556,7 @@ describe('javascript', function () {
55695556
{require: false},
55705557
);
55715558

5572-
assert.deepEqual(
5573-
calls,
5574-
shouldScopeHoist ? ['message1'] : ['message1', 'message'],
5575-
);
5559+
assert.deepEqual(calls, ['message1']);
55765560
assert.deepEqual(res.output, 'Message 1');
55775561
});
55785562

@@ -5626,12 +5610,7 @@ describe('javascript', function () {
56265610
{require: false},
56275611
);
56285612

5629-
assert.deepEqual(
5630-
calls,
5631-
shouldScopeHoist
5632-
? ['message1']
5633-
: ['message1', 'message', 'index2', 'index'],
5634-
);
5613+
assert.deepEqual(calls, ['message1']);
56355614
assert.deepEqual(res.output, 'Message 1');
56365615
});
56375616

@@ -5662,10 +5641,7 @@ describe('javascript', function () {
56625641
{require: false},
56635642
);
56645643

5665-
assert.deepEqual(
5666-
calls,
5667-
shouldScopeHoist ? ['other'] : ['other', 'index'],
5668-
);
5644+
assert.deepEqual(calls, ['other']);
56695645
assert.deepEqual(res.output, 'bar');
56705646
});
56715647

@@ -5696,10 +5672,7 @@ describe('javascript', function () {
56965672
{require: false},
56975673
);
56985674

5699-
assert.deepEqual(
5700-
calls,
5701-
shouldScopeHoist ? ['other'] : ['other', 'index'],
5702-
);
5675+
assert.deepEqual(calls, ['other']);
57035676
assert.deepEqual(res.output, 'bar');
57045677
});
57055678

@@ -5757,10 +5730,7 @@ describe('javascript', function () {
57575730
{require: false},
57585731
);
57595732

5760-
assert.deepEqual(
5761-
calls,
5762-
shouldScopeHoist ? ['other'] : ['other', 'index'],
5763-
);
5733+
assert.deepEqual(calls, ['other']);
57645734
assert.deepEqual(res.output, ['foo']);
57655735
});
57665736

@@ -5792,10 +5762,7 @@ describe('javascript', function () {
57925762
{require: false},
57935763
);
57945764

5795-
assert.deepEqual(
5796-
calls,
5797-
shouldScopeHoist ? ['other'] : ['other', 'index'],
5798-
);
5765+
assert.deepEqual(calls, ['other']);
57995766
assert.deepEqual(res.output, ['bar']);
58005767
});
58015768

@@ -5827,10 +5794,7 @@ describe('javascript', function () {
58275794
{require: false},
58285795
);
58295796

5830-
assert.deepEqual(
5831-
calls,
5832-
shouldScopeHoist ? ['other'] : ['other', 'index'],
5833-
);
5797+
assert.deepEqual(calls, ['other']);
58345798
assert.deepEqual(res.output, ['foo', 'bar']);
58355799
});
58365800

@@ -5858,16 +5822,11 @@ describe('javascript', function () {
58585822

58595823
let [v, async] = res;
58605824

5861-
assert.deepEqual(calls, shouldScopeHoist ? ['b'] : ['b', 'index']);
5825+
assert.deepEqual(calls, ['b']);
58625826
assert.deepEqual(v, 2);
58635827

58645828
v = await async();
5865-
assert.deepEqual(
5866-
calls,
5867-
shouldScopeHoist
5868-
? ['b', 'a', 'index', 'dynamic']
5869-
: ['b', 'index', 'a', 'dynamic'],
5870-
);
5829+
assert.deepEqual(calls, ['b', 'a', 'index', 'dynamic']);
58715830
assert.deepEqual(v.default, [1, 3]);
58725831
});
58735832

@@ -5893,10 +5852,7 @@ describe('javascript', function () {
58935852
{require: false},
58945853
);
58955854

5896-
assert.deepEqual(
5897-
calls,
5898-
shouldScopeHoist ? ['esm1'] : ['esm1', 'index'],
5899-
);
5855+
assert.deepEqual(calls, ['esm1']);
59005856
assert.deepEqual(res.output, 'Message 1');
59015857
});
59025858

@@ -5918,7 +5874,7 @@ describe('javascript', function () {
59185874
assert.deepStrictEqual(
59195875
new Set(b.getUsedSymbols(nullthrows(findAsset(b, 'commonjs.js')))),
59205876
// the exports object is used freely
5921-
new Set(['*', 'message1']),
5877+
new Set(shouldScopeHoist ? ['*', 'message1'] : ['message1']),
59225878
);
59235879
assert.deepStrictEqual(
59245880
new Set(
@@ -5958,7 +5914,7 @@ describe('javascript', function () {
59585914
assert.deepStrictEqual(
59595915
new Set(b.getUsedSymbols(nullthrows(findAsset(b, 'commonjs.js')))),
59605916
// the exports object is used freely
5961-
new Set(['*', 'message2']),
5917+
new Set(shouldScopeHoist ? ['*', 'message2'] : ['message2']),
59625918
);
59635919
}
59645920

@@ -5972,10 +5928,7 @@ describe('javascript', function () {
59725928
},
59735929
{require: false},
59745930
);
5975-
assert.deepEqual(
5976-
calls,
5977-
shouldScopeHoist ? ['commonjs'] : ['commonjs', 'index'],
5978-
);
5931+
assert.deepEqual(calls, ['commonjs']);
59795932
assert.deepEqual(res.output, 'Message 2');
59805933
});
59815934
});

‎packages/core/integration-tests/test/sourcemaps.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -453,7 +453,7 @@ describe('sourcemaps', function () {
453453
source: inputs[0],
454454
generated: raw,
455455
str: 'const local',
456-
generatedStr: 'let r',
456+
generatedStr: 'let o',
457457
sourcePath: 'index.js',
458458
});
459459

@@ -462,7 +462,7 @@ describe('sourcemaps', function () {
462462
source: inputs[0],
463463
generated: raw,
464464
str: 'local.a',
465-
generatedStr: 'r.a',
465+
generatedStr: 'o.a',
466466
sourcePath: 'index.js',
467467
});
468468

@@ -471,7 +471,7 @@ describe('sourcemaps', function () {
471471
source: inputs[1],
472472
generated: raw,
473473
str: 'exports.a',
474-
generatedStr: 'o.a',
474+
generatedStr: 'r.a',
475475
sourcePath: 'local.js',
476476
});
477477

@@ -480,7 +480,7 @@ describe('sourcemaps', function () {
480480
source: inputs[2],
481481
generated: raw,
482482
str: 'exports.count = function(a, b) {',
483-
generatedStr: 'o.count=function(e,n){',
483+
generatedStr: 'r.count=function(e,n){',
484484
sourcePath: 'utils/util.js',
485485
});
486486
});

‎packages/packagers/js/src/DevPackager.js

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,31 @@ export class DevPackager {
118118
if (this.bundleGraph.isDependencySkipped(dep)) {
119119
deps[specifier] = false;
120120
} else if (resolved) {
121-
deps[specifier] = this.bundleGraph.getAssetPublicId(resolved);
121+
let assetId = this.bundleGraph.getAssetPublicId(resolved);
122+
let resolution = assetId;
123+
124+
// Dependencies may be re-targeted to follow re-exports.
125+
// Pass these re-writes into the prelude.
126+
for (let [name, sym] of dep.symbols) {
127+
let rewritten = sym.meta?.rewritten;
128+
if (typeof rewritten === 'string') {
129+
let r =
130+
rewritten === name
131+
? [rewritten, assetId]
132+
: [rewritten, assetId, name];
133+
if (typeof resolution === 'string') {
134+
resolution = [r];
135+
} else {
136+
resolution.push(r);
137+
}
138+
}
139+
}
140+
141+
if (Array.isArray(deps[specifier]) && Array.isArray(resolution)) {
142+
deps[specifier].push(...resolution);
143+
} else {
144+
deps[specifier] = resolution;
145+
}
122146
} else {
123147
// An external module - map placeholder to original specifier.
124148
deps[specifier] = dep.specifier;
@@ -131,6 +155,18 @@ export class DevPackager {
131155
}
132156
}
133157

158+
// Simplify dependency resolutions when all symbols point to a single asset.
159+
for (let specifier in deps) {
160+
let resolution = deps[specifier];
161+
if (
162+
Array.isArray(resolution) &&
163+
resolution.length > 0 &&
164+
resolution.every(r => r.length === 2 && r[1] === resolution[0][1])
165+
) {
166+
deps[specifier] = resolution[0][1];
167+
}
168+
}
169+
134170
// Add dependencies for parcelRequire calls added by runtimes
135171
// so that the HMR runtime can correctly traverse parents.
136172
let hmrDeps = asset.meta.hmrDeps;

0 commit comments

Comments
 (0)