fix(proposal-decorators): only compile decorated fields with legacy decorators#18125
fix(proposal-decorators): only compile decorated fields with legacy decorators#18125Develop-KIM wants to merge 3 commits intobabel:mainbabel/babel:mainfrom Develop-KIM:legacy-decorators-only-compile-decorated-fieldsDevelop-KIM/babel:legacy-decorators-only-compile-decorated-fieldsCopy head branch name to clipboard
Conversation
…ecorators Legacy decorators depend on the class-properties transform to lower decorated fields, which lowers every field, private element and method of the class even when only one field is decorated. Without class-properties the decorated field is emitted as a `_initializerWarningHelper` call that throws at runtime. When class-properties is not enabled, lower only the decorated instance fields into the constructor and leave undecorated members as native class syntax. When it is enabled, the existing output and field initialization order are preserved. Fixes babel#14917 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
❌ Automation signalsActivity patterns show signs of automation. This is an automated analysis by AgentScan |
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/61889 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 955101f8e9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| | NodePath<t.ClassMethod> | ||
| | undefined; | ||
|
|
||
| injectInitialization(path, constructor, instanceFieldInitializers); |
There was a problem hiding this comment.
Preserve initializer order for mixed fields
When proposal-decorators runs without the class-properties transform, this injects every decorated field initializer into the constructor while leaving the other fields as native class fields. In a class such as a = log('a'); @dec b = log('b'); c = log('c'), the remaining native fields run before the constructor body, so the transformed order becomes a, c, b instead of a, b, c (and private fields after a decorated field are likewise initialized too early). This silently changes observable side effects and initialization errors whenever decorated and undecorated instance fields are mixed.
Useful? React with 👍 / 👎.
|
commit: |
…ators A native field initializer runs before the constructor body, so lowering only the decorated fields into the constructor reordered initialization when an undecorated field followed a decorated one. Relocate every instance field after the first decorated one into the constructor too, in source order; fields before it, methods and static members stay native. Fall back to the marker handshake when a computed key would need to move. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The previous version reordered initializers when an undecorated field followed a decorated one, since native fields run before the constructor body. Fixed by relocating every instance field after the first decorated one (public and private) into the constructor in source order. Fields before the first decorator, methods and statics stay native; a computed key that would need to move falls back to the marker handshake. Added a |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fa168fa386
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ), | ||
| ), | ||
| ); | ||
| node.value = null; |
There was a problem hiding this comment.
Keep private brands from being installed early
When a private field follows the first decorated field in the no-class-properties path, this clears the value but leaves a native #x; element in the class. Native private elements are installed before the constructor body, so a relocated earlier initializer such as @dec a = (#x in this); #x = 1; observes true instead of the source-order false (or reads undefined instead of throwing through a method) before #x's position is reached. Fresh evidence after the update is that this path leaves the declaration in place while moving only its value into the constructor.
Useful? React with 👍 / 👎.
… a decorator A native private field installs its brand with the class element, before the constructor body, so relocating an earlier initializer that reaches the private name (for example through a method) made it observe the brand too early: it read undefined instead of throwing. The brand cannot be deferred without the full private-field lowering that only the class-properties transform performs. Fall back to the class-properties handshake when a private instance field follows the first decorated field, the same way a computed key already does, so only undecorated public fields are relocated into the constructor. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8b2a7cdb72
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| t.callExpression(state.addHelper("defineProperty"), [ | ||
| t.thisExpression(), | ||
| computed || t.isLiteral(key) ? key : t.stringLiteral(key.name), | ||
| node.value || t.buildUndefinedNode(), |
There was a problem hiding this comment.
Preserve initializer scope when relocating fields
When a trailing undecorated field initializer references a name that is also bound by the constructor, moving the raw node.value into the constructor makes it resolve against constructor parameters/var bindings. Native field initializers do not have access to constructor scope, so class C { @dec x = 0; y = value; constructor(value) {} } should read an outer value or throw, but this transform makes y use the constructor argument. The class-features path passes a renamer to injectInitialization for this reason; this relocation path needs equivalent renaming/wrapping before injecting these initializers.
Useful? React with 👍 / 👎.
Legacy decorators depend on the class-properties transform to lower decorated
fields: a decorated field is emitted as an
_initializerWarningHelper(...)marker that class-properties turns into an
_initializerDefinePropertycall inthe constructor. When class-properties is enabled it lowers every field,
private element and method of the class. When it is not enabled (e.g.
targeting an engine with native class fields), the marker survives and throws at
runtime.
This makes legacy decorators work without the class-properties transform, along
the lines @nicolo-ribaudo suggested in the issue:
initializerDefineProperty), so a decorator that replaces the field with aprototype accessor keeps working — a native field initializer would create an
own data property that shadows the prototype getter. This is covered by the
new
getter-decoratorfixture.field that follows a decorated one would otherwise initialize too early. To
preserve the observable initialization order, every instance field after the
first decorated one (public and private) is relocated into the constructor in
source order. Fields before the first decorated field, methods and static
members stay as native class syntax. When a computed key would need to move
(its evaluation time would change), we keep the class-properties handshake.
field-initialization order are preserved unchanged (the marker/handshake path
is kept). Detection uses the class-features
fieldsfeature flag, so with@babel/preset-envthe field lowering still happens automatically when thetargets need it.
Before / after
With
@babel/plugin-proposal-decorators(legacy) and no class-properties, thispreviously emitted the throwing
_initializerWarningHelper. It now producesworking output that keeps the members that can stay native (fields before the
decorator, the method, the private field declaration) as class syntax, while
initializing the decorated field and the fields that follow it from the
constructor in source order:
An existing fixture in
@babel/plugin-transform-typescript(
class/abstract-class-decorated-parameter, a real mobx@observablefield)went from emitting the throwing
_initializerWarningHelperto workingconstructor initialization.
Disclosure: this change was written with AI assistance (see the
Co-Authored-Bytrailer on the commits). I've reviewed and can walk through every line.