environmentVisitor should skip decorator expressions#14371
environmentVisitor should skip decorator expressions#14371nicolo-ribaudo merged 12 commits intobabel:mainfrom
Conversation
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/51986/ |
| skipAndrequeueComputedKeysAndDecorators(path); | ||
| }, | ||
| } as Visitor<PluginPass>; | ||
| } as Visitor<any>; |
There was a problem hiding this comment.
Does Visitor<unknown> work here? If it does, it's safer since it doesn't "disable" type checking.
| const keys = VISITOR_KEYS[path.type]; | ||
| for (const key of keys) { | ||
| if (key !== "key") path.skipKey(key); | ||
| function skipAndrequeueComputedKeysAndDecorators( |
There was a problem hiding this comment.
| function skipAndrequeueComputedKeysAndDecorators( | |
| function skipAndRequeueComputedKeysAndDecorators( |
(or maybe something with less words like skipInternalContext or skipContents works too).
There was a problem hiding this comment.
I think skipInternalContext and skipContents are a bit vague since they are not formally defined in the spec. Maybe skipAndRequeueKeyAndDecorator? I also thought about skipParamAndBodyAndInitializer but it seems no better than skipAndRequeueKeyAndDecorator.
| const keys = VISITOR_KEYS[path.type]; | ||
| for (const key of keys) { | ||
| if (key !== "key") path.skipKey(key); | ||
| function skipAndrequeueComputedKeysAndDecorators(path: NodePath) { |
There was a problem hiding this comment.
Can we import the logic from the helper?
path.isMethod() ? skipAndRequeueComputedKeysAndDecorators(path) : path.skip();5717af3 to
b35e893
Compare
| const keys = VISITOR_KEYS[path.type]; | ||
| for (const key of keys) { | ||
| if (key !== "key") path.skipKey(key); | ||
| export function requeueComputedKeyAndDecorator( |
There was a problem hiding this comment.
| export function requeueComputedKeyAndDecorator( | |
| export function requeueComputedKeyAndDecorators( |
I like how the new behavior gave us a more readable name!
8f8ccae to
34fb84c
Compare
|
@JLHwung It has been two months since your last update to this PR, time for a self-review? 🙏 |
34fb84c to
791d2f4
Compare
| "ClassProperty|ClassPrivateProperty"( | ||
| path: NodePath<t.ClassProperty | t.ClassPrivateProperty>, | ||
| ) { | ||
| Property(path) { |
There was a problem hiding this comment.
The visitor now hooks on Property alias so that it supports older @babel/types without ClassAccessorProperty definitions.
|
|
||
| babelHelpers.classCallCheck(this, Outer); | ||
| _dec = _this = _super.call(this) | ||
| _dec = _this = _super.call(this); |
There was a problem hiding this comment.
I don't know why this line is changed after rebasing. Maybe a generator update?
|
|
||
| const [newPath] = element.replaceWith(newField); | ||
| addProxyAccessorsFor(newPath, key, newId, element.node.computed); | ||
| addProxyAccessorsFor(newPath, key, newId, computed); |
There was a problem hiding this comment.
This is a bug fix, the element.node.computed is of the replacement node, which is always false.
Fixes #1, Fixes #2In this PR we reimplement
skipAllButComputedKeyso we can remove@babel/typesdeps of the environment-visitor helper. In the 2nd commit we expand similar logic to decorator expressions as well. Note that the test cases are already passing in main because the decorator transforms will hoist the expression. In the last commit we apply similar changes to the renamer.