Conversation
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/51444/ |
There was a problem hiding this comment.
LGTM pending the message update. I'll mark this as "spec compliance", since we usually use that when we make a transform more restrictive to match the spec (since there is a very small chance that it will break someone, if they are using it in a non-spec-compliant way).
683d456 to
e32c532
Compare
| initializer = newValue; | ||
| } else if (kind === 1 /* ACCESSOR */) { | ||
| initializer = newValue.initializer; | ||
| initializer = newValue.initialize; |
There was a problem hiding this comment.
Since we are renaming a property here, I wonder if it makes sense to also support the old one with a warning:
if ((initializer = newValue.init) == null && (initializer = newValue.initializer) && typeof console !== "undefined") {
console.warn(".initializer has been renamed to .init as of March 2022")
}@pzuraq This idea mostly came up while I was reviewing #14353, since it's not just a "we implemented the spec wrongly" anymore but a "the proposal had a change that made all the accessor initializers usages invalid".
There was a problem hiding this comment.
that seems reasonable to me, will update
There was a problem hiding this comment.
Also, feel free to review/approve this PR since you are the "expert in the field" for decorators! However, I'd like to wait merging this until #14353 is ready so that they can go in the same release.
|
This PR is good as long as it's followed shortly with the changes in #14353 |
Please review this PR by commits. Notable changes:
context.addInitializerare now invoked without any params, previously we passed down the class instance or the decorated class.initializereturned from the accessor decorator is now respected, previously we accessed.initializer.TypeErrorper spec.