fix(transformer/typescript): emit class fields for parameter properties#21831
fix(transformer/typescript): emit class fields for parameter properties#21831
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
Merging this PR will not alter performance
Comparing Footnotes
|
8751841 to
4a64d7e
Compare
|
Overrode two Babel test outputs to match oxc's new output. They are correct — the reason is that oxc defaults to |
c0fa4c3 to
bf76f6c
Compare
Merge activity
|
…es (#21831) ## Summary Fixes a long-standing inconsistency in the TypeScript transformer's handling of constructor parameter properties under the documented default (`useDefineForClassFields: true` per [`compiler_assumptions.rs:137-139`](https://github.com/oxc-project/oxc/blob/main/crates/oxc_transformer/src/compiler_assumptions.rs#L137-L139)). The transformer already emitted TS-true-aligned output for **declared** fields (kept `boom: number;` as `boom;`) but silently dropped the field declaration for **parameter properties**, so downstream tools that re-analyze the transformed AST saw the property only as a `this.*` write, not as a declared class field. ```ts // input class Foo { boom: number; // declared field constructor(public foo) {} // parameter property } ``` ```js // before — inconsistent class Foo { boom; // ✅ kept (TS-true) constructor(foo) { // ❌ missing `foo;` (TS-false-style for params only) this.foo = foo; } } // after — consistent class Foo { foo; // ✅ now matches `boom;` boom; constructor(foo) { this.foo = foo; } } ``` This is **not a policy change** — the documented default has always been `useDefineForClassFields: true` (set both `set_public_class_fields` and `remove_class_fields_without_initializer` to `true` to opt into `false` semantics). The fix just closes the missing case for parameter properties. ## Implementation - Add `add_parameter_property_fields` to the `enter_class` path that runs when `set_public_class_fields` is `false`, mirroring the existing branch that calls `transform_class_fields` when it is `true`. - Skip parameter properties whose name matches an existing instance field on the same class (matches tsc's dedup). Static, private, and `declare`-only fields are different bindings, so they don't suppress the new field. - Early-bail before allocating the dedup set when the class has no parameter properties — keeps the transformer-allocations snapshot at baseline. No new option is added; the existing `set_public_class_fields` assumption already maps cleanly to `useDefineForClassFields` (the playground's `useDefineForClassFields: true` already toggles `set_public_class_fields = false`). Closes #21600 Closes #21831
bf76f6c to
c59c199
Compare
### 💥 BREAKING CHANGES - 0ffbe0d allocator: [**BREAKING**] Remove `Allocator::end_ptr` method (#21871) (overlookmotel) ### 🚀 Features - 9593ec8 transformer/jsx: Add jsxDEV source metadata for fragments (#21932) (Ido Rosenthal) ### 🐛 Bug Fixes - 429deac napi/parser: Export `visitorKeys` from `wasm` entrypoint (#21996) (NullVoxPopuli) - e852911 codegen: Preserve legal comments orphaned by upstream passes (#21575) (Dunqing) - e3399ec transformer/class-properties: Preserve RHS in logical-assignment to static private field (#21950) (Dunqing) - c59c199 transformer/typescript: Emit class fields for parameter properties (#21831) (Dunqing) - aaabde4 parser: Attach legal comments to following token (#21670) (Dunqing) ### ⚡ Performance - 0bf0cb9 allocator: Per-platform `Arena::new_fixed_size` implementations (#22088) (overlookmotel) ### 📚 Documentation - 62ec410 allocator: Correct doc comment for `Allocator::from_raw_parts` (#22093) (overlookmotel) - 3e152c6 allocator: Correct typos in comments (#22092) (overlookmotel) - e220855 allocator: Correct doc comment for `Allocator::set_cursor_ptr` (#21866) (overlookmotel) --------- Co-authored-by: Boshen <1430279+Boshen@users.noreply.github.com> Co-authored-by: Cameron Clark <cameron.clark@hey.com>

Summary
Fixes a long-standing inconsistency in the TypeScript transformer's handling of constructor parameter properties under the documented default (
useDefineForClassFields: truepercompiler_assumptions.rs:137-139).The transformer already emitted TS-true-aligned output for declared fields (kept
boom: number;asboom;) but silently dropped the field declaration for parameter properties, so downstream tools that re-analyze the transformed AST saw the property only as athis.*write, not as a declared class field.This is not a policy change — the documented default has always been
useDefineForClassFields: true(set bothset_public_class_fieldsandremove_class_fields_without_initializertotrueto opt intofalsesemantics). The fix just closes the missing case for parameter properties.Implementation
add_parameter_property_fieldsto theenter_classpath that runs whenset_public_class_fieldsisfalse, mirroring the existing branch that callstransform_class_fieldswhen it istrue.declare-only fields are different bindings, so they don't suppress the new field.No new option is added; the existing
set_public_class_fieldsassumption already maps cleanly touseDefineForClassFields(the playground'suseDefineForClassFields: truealready togglesset_public_class_fields = false).Closes #21600
Closes #21831