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

Add parcel-query function to locate a symbol by local name #8452

Merged
merged 4 commits into from Dec 13, 2022

Conversation

lettertwo
Copy link
Member

Based on the output for findAsset.

Some examples of how it works:

An imported symbol:

> .findAssetWithSymbol $8f94277571a64835$export$e08c7d021b829b7a
bRRst node_modules/stylis/dist/stylis.mjs
imported as prefixer from node_modules/stylis/src/Enum.js

A declared symbol:

> .findAssetWithSymbol $3780a8199c4f575f$var$mapping
4LrcW node_modules/@parcel/runtime-js/lib/helpers/bundle-manifest.js
possibly defined as mapping

@lettertwo lettertwo changed the title Add parcel-query function to locate a symbol by local name. Add parcel-query function to locate a symbol by local name Sep 7, 2022
@parcel-benchmark
Copy link

parcel-benchmark commented Sep 7, 2022

Benchmark Results

Kitchen Sink ✅

Timings

Description Time Difference
Cold 1.51s -25.00ms
Cached 342.00ms -42.00ms 🚀

Cold Bundles

No bundle changes detected.

Cached Bundles

Bundle Size Difference Time Difference
dist/legacy/index.html 826.00b +0.00b 516.00ms -39.00ms 🚀
dist/modern/index.html 749.00b +0.00b 515.00ms -40.00ms 🚀

React HackerNews ✅

Timings

Description Time Difference
Cold 9.80s -103.00ms
Cached 456.00ms -5.00ms

Cold Bundles

No bundle changes detected.

Cached Bundles

No bundle changes detected.

AtlasKit Editor ✅

Timings

Description Time Difference
Cold 1.77m +207.00ms
Cached 2.30s -16.00ms

Cold Bundles

No bundle changes detected.

Cached Bundles

No bundle changes detected.

Three.js ✅

Timings

Description Time Difference
Cold 7.09s -82.00ms
Cached 277.00ms +5.00ms

Cold Bundles

No bundle changes detected.

Cached Bundles

No bundle changes detected.

Click here to view a detailed benchmark overview.


// If the asset couldn't be found by searching for the id,
// search for the local name in asset used symbols.
if (asset == null) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you actually run into such a case? I can't think of a situation where the first search would fail

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you're right that this shouldn't be possible, but if there is an undefined symbol at runtime (due to a bug in Parcel), it likely won't be discoverable via the asset graph, but might still be discoverable in used symbols. (I have one such case that I am investigating and can share).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's the case i mentioned: #8468

Base automatically changed from parcel-query to v2 December 13, 2022 17:57
First pass optimistically looks for a matching id.
Second pass will examine asset symbols, which could be much
slower if there are a lot of assets and symbols.
@lettertwo lettertwo force-pushed the parcel-query-find-asset-with-symbol branch from 2408949 to 518a398 Compare December 13, 2022 18:43
Copy link
Contributor

@AGawrys AGawrys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is super useful in debugging issues that manifest as missing symbol (even if its not a Parcel bug, but a user's mishap)

@mischnic mischnic merged commit ed1e231 into v2 Dec 13, 2022
@mischnic mischnic deleted the parcel-query-find-asset-with-symbol branch December 13, 2022 21:03
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.

None yet

4 participants