-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Bump swc #8029
Bump swc #8029
Conversation
Can the |
I just had the exact same thing happen with Though I'm not actually sure if that commit was correct or just made things worse... |
So it turns out that |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this ready or is the first checkbox in the description still pending?
This whole global_mark/global_syntax situation is still not fully resolved (which is also the problem for #7911). Maybe I can add some hacks to make it work for now |
This should now be correct (and fix that one issue), but it introduces one extra So we'll want to benchmark if this makes a difference. If it's too slow, we could alternatively create a custom visitor that collects all top level variables into a set and then check against that instead of |
Last blocker for the tests: swc-project/swc#4608 |
Build ak-editor is equally fast |
Benchmark ResultsKitchen Sink 🚨
Timings
Cold BundlesNo bundles found, this is probably a failed build... Cached BundlesNo bundles found, this is probably a failed build... React HackerNews ✅
Timings
Cold Bundles
Cached Bundles
AtlasKit Editor ✅
Timings
Cold Bundles
Cached Bundles
Three.js ✅
Timings
Cold Bundles
Cached Bundles
|
hygiene(), | ||
resolver(unresolved_mark, global_mark, false) | ||
)); | ||
(collect_decls(&module), module) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So if the node replacements pass doesn't run we do this twice in a row? Also, are there more cases where these extra passes can be skipped (e.g. if no preset-env)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've made our replacement passes use the "incorrect" global syntax context directly (which is what will be the case after the resolver reruns anyway). So now it's only needed for preset_env.
collect_decls
has to always be rerun here because of line 3:
// Flush (JsWord, SyntaxContexts) into unique names and reresolve to
// set global_mark for all nodes, even generated ones.
// - This changes the syntax context ids and therefore invalidates decls
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm I guess the real question is why the first one exists before the node replacements pass, or why the node pass runs after preset-env rather than before?
I guess some tests are failing now? |
Closes #7911