Skip to content

Commit

Permalink
Don't retarget dependencies if a symbol is imported multiple times wi…
Browse files Browse the repository at this point in the history
…th different local names (#8738)
  • Loading branch information
mischnic committed Jan 3, 2023
1 parent 90a886f commit eba8dd6
Show file tree
Hide file tree
Showing 7 changed files with 44 additions and 16 deletions.
12 changes: 9 additions & 3 deletions packages/core/core/src/BundleGraph.js
Original file line number Diff line number Diff line change
Expand Up @@ -225,12 +225,12 @@ export default class BundleGraph {
}

if (
// Only perform rewriting when there is an imported symbol
// Only perform retargeting when there is an imported symbol
// - If the target is side-effect-free, the symbols point to the actual target and removing
// the original dependency resolution is fine
// - Otherwise, keep this dependency unchanged for its potential side effects
node.usedSymbolsUp.size > 0 &&
// Only perform rewriting if the dependency only points to a single asset (e.g. CSS modules)
// Only perform retargeting if the dependency only points to a single asset (e.g. CSS modules)
!hasAmbiguousSymbols &&
// It doesn't make sense to retarget dependencies where `*` is used, because the
// retargeting won't enable any benefits in that case (apart from potentially even more
Expand All @@ -243,7 +243,13 @@ export default class BundleGraph {
// or
// (parcelRequire("...")).then((a)=>a);
// if the reexporting asset did `export {a as b}` or `export * as a`
node.value.priority === Priority.sync
node.value.priority === Priority.sync &&
// For every asset, no symbol is imported multiple times (with a different local name).
// Don't retarget because this cannot be resolved without also changing the asset symbols
// (and the asset content itself).
[...targets].every(
([, t]) => new Set([...t.values()]).size === t.size,
)
) {
// TODO adjust sourceAssetIdNode.value.dependencies ?
let deps = [
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import * as ns from "./library/a";

output = [ns, ns.value1, ns.value2];
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { value1, value2 } from './b.js';
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { value1, value1 as value2 } from "./c";
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const value1 = 123;
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"sideEffects": false
}
39 changes: 26 additions & 13 deletions packages/core/integration-tests/test/javascript.js
Original file line number Diff line number Diff line change
Expand Up @@ -7079,19 +7079,6 @@ describe('javascript', function () {
assert.deepEqual(res.output, 'bar');
});

it('supports reexports via variable declaration (unused)', async function () {
let b = await bundle(
path.join(
__dirname,
'/integration/scope-hoisting/es6/side-effects-re-exports-rename-var-unused/index.js',
),
options,
);

let res = await run(b, {}, {require: false});
assert.deepEqual((await res.output).foo, 'foo');
});

it('supports named and renamed reexports of the same asset (named used)', async function () {
let b = await bundle(
path.join(
Expand Down Expand Up @@ -7126,6 +7113,32 @@ describe('javascript', function () {
assert.deepEqual(res.output, 'bar');
});

it('supports named and renamed reexports of the same asset (namespace used)', async function () {
let b = await bundle(
path.join(
__dirname,
'/integration/scope-hoisting/es6/side-effects-re-exports-rename-same/index.js',
),
options,
);

let res = await run(b, null, {require: false});
assert.deepEqual(res.output, [{value1: 123, value2: 123}, 123, 123]);
});

it('supports reexports via variable declaration (unused)', async function () {
let b = await bundle(
path.join(
__dirname,
'/integration/scope-hoisting/es6/side-effects-re-exports-rename-var-unused/index.js',
),
options,
);

let res = await run(b, {}, {require: false});
assert.deepEqual((await res.output).foo, 'foo');
});

it('supports named and namespace exports of the same asset (named used)', async function () {
let b = await bundle(
path.join(
Expand Down

0 comments on commit eba8dd6

Please sign in to comment.