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

Use placeholder expression when replacing unused symbols #8358

Merged
merged 4 commits into from
Aug 2, 2022

Conversation

mischnic
Copy link
Member

@mischnic mischnic commented Aug 1, 2022

This is a case where our currently limited form of inner-module tree shaking determined b and b2 to be unused (since only a was imported). But then we still have the var $id$export$id = $id$import$b; declaration in the output.

import * as a from "./a";
import b from "./b";
export { a, b };
export var b2 = b;

The RHS was previously replaced with a reference to an asset that doesn't exist (because is was skipped).

parcelRequire.register("9lh8q", function(module, exports) {
	$parcel$export(module.exports, "a", () => (parcelRequire("e0WFk")));
	var $e0WFk = parcelRequire("e0WFk");

        // unused `export var b2 = b;`
	var $6cd35dda6492a1a0$export$2ec5befb9e6c97d4 = (0, $3nA9q.default);
});

After:

parcelRequire.register("9lh8q", function(module, exports) {
	$parcel$export(module.exports, "a", () => (parcelRequire("e0WFk")));
	var $e0WFk = parcelRequire("e0WFk");

	var $6cd35dda6492a1a0$export$2ec5befb9e6c97d4 = (0, {});
});

(Also note that this is not the common case, for unused export { ... } declarations, there is nothing in the asset and the packager doesn't generate the $parcel$export call in the first place. So there is no problematic placeholder as in the added test case).

@parcel-benchmark
Copy link

Benchmark Results

Kitchen Sink ✅

Timings

Description Time Difference
Cold 1.90s -28.00ms
Cached 390.00ms -18.00ms

Cold Bundles

No bundle changes detected.

Cached Bundles

No bundle changes detected.

React HackerNews ✅

Timings

Description Time Difference
Cold 11.87s -49.00ms
Cached 523.00ms -18.00ms

Cold Bundles

No bundle changes detected.

Cached Bundles

No bundle changes detected.

AtlasKit Editor ✅

Timings

Description Time Difference
Cold 2.07m +206.00ms
Cached 3.13s +262.00ms ⚠️

Cold Bundles

No bundle changes detected.

Cached Bundles

No bundle changes detected.

Three.js ✅

Timings

Description Time Difference
Cold 8.54s -79.00ms
Cached 326.00ms -26.00ms 🚀

Cold Bundles

No bundle changes detected.

Cached Bundles

No bundle changes detected.

Click here to view a detailed benchmark overview.

@devongovett
Copy link
Member

Good to merge now @mischnic?

@devongovett devongovett merged commit 1a96d6d into v2 Aug 2, 2022
@devongovett devongovett deleted the unused-var-reexport-assignment branch August 2, 2022 17:36
gorakong pushed a commit that referenced this pull request Nov 3, 2022
* upstream/v2: (22 commits)
  Cross compile toolchains are built into docker image already
  Also fix release build
  Update centos node version
  v2.7.0
  Changelog for v2.7.0
  Use placeholder expression when replacing unused symbols (#8358)
  Lint (#8359)
  Add support for errorRecovery option in @parcel/transformer-css (#8352)
  VS Code Extension for Parcel (#8139)
  Add multi module compilation for elm (#8076)
  Bump terser from 5.7.2 to 5.14.2 (#8322)
  Bump node-forge from 1.2.1 to 1.3.0 (#8271)
  allow cjs config files on type module projects (#8253)
  inject script for hmr when there is only normal script in html (#8330)
  feat: support react refresh for @emotion/react (#8205)
  Update index.d.ts (#8293)
  remove charset from jsloader script set (#8346)
  Log resolved targets in verbose log level (#8254)
  Fix missing module in large app from Experimental Bundler (#8303)
  [Symbol Propagation] Non-deterministic bundle hashes (#8212)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants