-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
refactor(es/compat): Use special span instead of passing static_blocks_mark
#9725
Conversation
🦋 Changeset detectedLatest commit: 586b38b The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
CodSpeed Performance ReportMerging #9725 will degrade performances by 8.27%Comparing Summary
Benchmarks breakdown
|
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.
What if plugin authors create a private name with dummy spans?
I think some plugins related to decorators may generate some private names, and if it's the case, most of the plugin authors will simply use dummy span (at least initially)
Unfortunately, we have already used a similar trick here swc/crates/swc_ecma_compat_es2015/src/classes/mod.rs Lines 631 to 636 in a417ff4
Or here swc/crates/swc_ecma_compat_es2015/src/classes/constructor.rs Lines 23 to 28 in 49a8b3e
|
Hmm. Makes sense. I'll review this PR later today. |
Or should we add another special span? By the way: We added PURE_SP last time, but we didn't add the documentation properly. I personally prefer to use dummy span directly. |
I didn't think deeply enough while writing the We may introduce another special flag in bytepos like Also, can you add a bit of documentation? |
static_blocks_mark
static_blocks_mark
Description:
Let's make an assumption in the code: the use of dummy span as a private class field name is generated by us, it is there simply because it is necessary to maintain the order of side effect execution, its name is not referenced elsewhere, so we can safely remove its name.
This is a breaking change for Rust users.
BREAKING CHANGE:
Related issue (if exists):