Skip to content

Commit

Permalink
Scope Hoisting: Don't insert unused requires that aren't registered a…
Browse files Browse the repository at this point in the history
…nywhere (#7764)
  • Loading branch information
gorakong committed Apr 21, 2022
1 parent 9b7a5d2 commit 2f39f29
Show file tree
Hide file tree
Showing 7 changed files with 35 additions and 2 deletions.
@@ -0,0 +1,3 @@
import {foo} from './library';

output = foo;
@@ -0,0 +1,2 @@
export const unusedFoo = 'unusedFoo';
export { foo } from './other';
@@ -0,0 +1 @@
export const foo = 'foo';
@@ -0,0 +1,3 @@
{
"sideEffects": false
}
@@ -0,0 +1,3 @@
export { foo, unusedFoo } from './foo';

eval('')
12 changes: 12 additions & 0 deletions packages/core/integration-tests/test/scope-hoisting.js
Expand Up @@ -5336,4 +5336,16 @@ describe('scope hoisting', function () {
let output = await run(b);
assert.strictEqual(output, 'bar foo bar');
});

it("not insert unused requires that aren't registered anywhere", async function () {
let b = await bundle(
path.join(
__dirname,
'/integration/scope-hoisting/es6/unused-require/index.js',
),
);

let output = await run(b);
assert.strictEqual(output, 'foo');
});
});
13 changes: 11 additions & 2 deletions packages/packagers/js/src/ScopeHoistingPackager.js
Expand Up @@ -760,9 +760,18 @@ ${code}
let staticExports = resolvedAsset.meta.staticExports !== false;
let publicId = this.bundleGraph.getAssetPublicId(resolvedAsset);

// If the rsolved asset is wrapped, but imported at the top-level by this asset,
// If the resolved asset is wrapped, but imported at the top-level by this asset,
// then we hoist parcelRequire calls to the top of this asset so side effects run immediately.
if (isWrapped && dep && !dep?.meta.shouldWrap && symbol !== false) {
if (
isWrapped &&
dep &&
!dep?.meta.shouldWrap &&
symbol !== false &&
// Only do this if the asset is part of a different bundle (so it was definitely
// parcelRequire.register'ed there), or if it is indeed registered in this bundle.
(!this.bundle.hasAsset(resolvedAsset) ||
!this.shouldSkipAsset(resolvedAsset))
) {
let hoisted = this.hoistedRequires.get(dep.id);
if (!hoisted) {
hoisted = new Map();
Expand Down

0 comments on commit 2f39f29

Please sign in to comment.