Register symbols even without scopehoisting#7222
Merged
Merged
Conversation
|
This pull request has been linked to and will mark 1 task as "Done" when merged:
|
mischnic
force-pushed
the
noscopehoist-symbols-swc
branch
from
16d9665 to
e30ea1a
Compare
Benchmark ResultsKitchen Sink ✅
Timings
Cold BundlesNo bundle changes detected. Cached Bundles
React HackerNews ✅
Timings
Cold Bundles
Cached Bundles
AtlasKit Editor ✅
Timings
Cold Bundles
Cached Bundles
Three.js ✅
Timings
Cold BundlesNo bundle changes detected. Cached BundlesNo bundle changes detected. |
mischnic
force-pushed
the
noscopehoist-symbols-swc
branch
3 times, most recently
from
1c6de95 to
460d0ee
Compare
devongovett
reviewed
Nov 12, 2021
| includeNodeModules: { | ||
| '@parcel/transformer-js': true, | ||
| } else { | ||
| if (symbol_result) { |
Member
There was a problem hiding this comment.
Is the following duplicated? Should we combine hoist_result and symbol_result somehow?
Member
|
Would be great to test this in a large app too. cc @lettertwo @thebriando @AGawrys @gorakong |
mischnic
force-pushed
the
noscopehoist-symbols-swc
branch
from
460d0ee to
0a38600
Compare
devongovett
approved these changes
Nov 15, 2021
devongovett
left a comment
devongovett
left a comment
Member
There was a problem hiding this comment.
Seems good overall. One question
| exports: collect | ||
| .exports | ||
| .into_iter() | ||
| .map( |
Member
There was a problem hiding this comment.
What's the reason to map these to a different struct with (almost) the same shape? Should we just expose the original? The less copying the better...
Member
Author
There was a problem hiding this comment.
That would be better, but they all contain IdentId (so (JsWord, SyntaxContext)) and the letter prevents serialization.
devongovett
approved these changes
Nov 16, 2021
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This enables
Known limitations for now:
Hoistvisitor, notCollect)Close T-733
Testing an app in dev/serve mode with
import { Provider, defaultTheme, Button } from "@adobe/react-spectrum":(so before, every component was transformed and bundled. now it's only the ones that are actually imported)
IIRC in #5528, the performance improvement more significant because the JS transformer pipeline was slower.
For https://github.com/mischnic/parcel-plugin-browser, many functions in
date-fnsandreact-usenow are skipped. The dev bundle went from 1.92mb to 1.31mb, the sourcemap from 3.04mb to 1.96mb. Cold startup into dev goes from 2.2s to 1.8s