Support multiple static blocks #12738
Conversation
JLHwung
added
PR: Spec Compliance 👓
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/44285/ |
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit e002f53:
|
JLHwung
force-pushed
the
multiple-static-blocks
branch
from
ddbc630
to
0c990d4
Compare
JLHwung
force-pushed
the
multiple-static-blocks
branch
from
0c990d4
to
d661b37
Compare
| @@ -1528,6 +1528,5 @@ export type ParseSubscriptState = { | |||
|
|
|||
| export type ParseClassMemberState = {| | |||
| hadConstructor: boolean, | |||
| hadStaticBlock: boolean, | |||
There was a problem hiding this comment.
The parser types are not exported, so we are free to refactor here.
| return; | ||
| for (const path of body) { | ||
| if (!path.isStaticBlock()) continue; | ||
| const staticBlockPrivateId = generateUid(scope, privateNames); |
There was a problem hiding this comment.
The inserted unique staticBlockPrivateId may accidentally shadow private id defined on upper levels:
class C {
static #_;
constructor() {
class D {
static {
C.#_ = 42;
}
}
}
}The injected #_ = AIIFE(static block) will shadow #_ defined on C. Consider reuse the privateNameVisitorFactory in @babel/helper-create-class-features-plugin.
There was a problem hiding this comment.
Why don't we just use the filename + source location to generate a private name?
There was a problem hiding this comment.
That does not really solve the mentioned issue, just makes it way less likely to happen. I don't think this is a blocker and we can address that in another PR.
There was a problem hiding this comment.
We should probably consider tracking private identifiers in @babel/traverse's generateUid.
There was a problem hiding this comment.
Or we provide a new generateUniquePrivateKeyAPI since plain identifier does not conflict with private identifiers.
|
Let's do #12738 (comment) in a separate PR, since it happens regardless of multiple static blocks. |
JLHwung
force-pushed
the
multiple-static-blocks
branch
from
d661b37
to
e002f53
Compare
github-actions
bot
added
the
outdated
I will not mark this PR as ready until the upstream PR is merged.
The integration test about
new.targetis disabled because of #12737. Yet the test ofstatic-blocksis still valid because we transform static blocks to static private field initializers.