Skip to content

fix(transformer/legacy-decorator): preserve accessor type annotation for emitDecoratorMetadata#21966

Draft
Dunqing wants to merge 5 commits intomainfrom
fix-transformer-accessor-emit-decorator-metadata
Draft

fix(transformer/legacy-decorator): preserve accessor type annotation for emitDecoratorMetadata#21966
Dunqing wants to merge 5 commits intomainfrom
fix-transformer-accessor-emit-decorator-metadata

Conversation

@Dunqing
Copy link
Copy Markdown
Member

@Dunqing Dunqing commented Apr 30, 2026

Fixes bug 7 of #21922.

The bug

@dec accessor x: T always emitted design:type = Object regardless of T:

class Entity {
  @dec accessor name: string = "";   // tsc: design:type = String
  @dec accessor count: number = 0;   // tsc: design:type = Number
}

Reflect.getMetadata("design:type", entity, "name") returned Object instead of String. Frameworks reading design:type (NestJS Swagger, class-validator, TypeORM column metadata) silently degraded for every typed accessor field.

Cause

lower_accessor_properties (in enter_class) rewrites

@dec accessor x: T = init;

into three members — a private storage field, a synthesized getter, and a synthesized setter — and transfers the decorators to the getter so the existing decorator pipeline picks them up. But it dropped the type annotation: the synth getter was constructed with return_type: NONE.

The metadata pass runs later and takes the getter branch:

if method.kind.is_get() {
    (self.serialize_type_annotation(method.value.return_type.as_ref(), ctx), None)
}

return_type is None because lowering discarded it, so serialize_type_annotation correctly fell through to Object for what it sees as an untyped getter.

The metadata pass's enter_accessor_property dispatch exists, but it never fires because the AccessorProperty no longer exists by the time traversal reaches the class body.

Fix

Stop dropping the type annotation. In lower_accessor_properties:

let decorators = std::mem::replace(&mut accessor.decorators, ctx.ast.vec());
let type_annotation = accessor.type_annotation.take();

Thread type_annotation through create_accessor_method to the getter (setter passes None) and attach it as the synth function's return_type. The existing getter-metadata path then serializes it correctly.

The lowered AST is now self-consistent — the synth getter for accessor x: T reads as a getter that returns T, which is what it semantically is.

Out of scope (architectural limit)

type Uuid = string; @dec accessor x: Uuid still emits Object, same as for non-accessor properties. Resolving type-alias references to their primitive constituent requires a name-based alias-resolution pass (the same one that tsc's checker provides via getTypeReferenceSerializationKind). That's bug 1 of #21922 and a separate change from this fix; flagged here so reviewers don't mistake it for an omission.

Test plan

  • New conformance fixture oxc/metadata/accessor covers primitives, untyped (correctly stays Object), string[]Array, and static accessor
  • cargo test -p oxc_transformer — 31 + 5 passed
  • cargo run -p oxc_transform_conformance -- --filter legacy-decorators — 73 pass / 23 fail (baseline 72/24, +1 new fixture, 0 regressions)
  • cargo fmt -p oxc_transformer

Copy link
Copy Markdown
Member Author

Dunqing commented Apr 30, 2026


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent changes, fast-track this PR to the front of 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.

@Dunqing Dunqing changed the title fix(transformer): preserve accessor type annotation for emitDecoratorMetadata fix(transformer/legacy-decorator): preserve accessor type annotation for emitDecoratorMetadata Apr 30, 2026
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Apr 30, 2026

Merging this PR will not alter performance

✅ 44 untouched benchmarks
⏩ 7 skipped benchmarks1


Comparing fix-transformer-accessor-emit-decorator-metadata (192da8b) with main (079cfdd)

Open in CodSpeed

Footnotes

  1. 7 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

Dunqing and others added 2 commits April 30, 2026 09:50
…Metadata

`@dec accessor x: T` previously emitted `design:type = Object` regardless of
`T`. Lowering rewrites the accessor into a private storage field plus a
synthesized getter/setter pair and transfers the decorators to the getter,
but it dropped the type annotation. The metadata pass then read the synth
getter's empty `return_type` and fell through to `Object` for every typed
accessor field — including primitives the metadata serializer handles
correctly elsewhere.

Route `accessor.type_annotation` onto the synth getter's `return_type` so
the existing getter path serializes it as `design:type`. The synth setter
(no decorators, no metadata) keeps `None`. `Reflect.getMetadata("design:type", ...)`
now returns the correct constructor for `string` / `number` / `boolean` / etc.

Refs #21922 (bug 7).

Co-Authored-By: Claude <noreply@anthropic.com>
The `declare function` form triggers a pre-existing semantic-data
inconsistency where the transformer strips the declaration but does not
remove `dec` from the post-transform scoping data, causing a "Bindings
mismatch" between the transformed-AST scoping and a freshly-rebuilt
analysis. Use a real function declaration to avoid the unrelated issue.

Also refresh `oxc.snap.md` and `oxc_exec.snap.md` to reflect the new
fixture passing both the unit and exec runs (226/373 and 14/16).

Co-Authored-By: Claude <noreply@anthropic.com>
@Dunqing Dunqing force-pushed the fix-transformer-accessor-emit-decorator-metadata branch from bea18eb to fc921b8 Compare April 30, 2026 01:51
Dunqing and others added 3 commits April 30, 2026 10:02
Three snapshots needed updating for the type-annotation-preservation
change in `lower_accessor_properties`:

- `transform_conformance/snapshots/oxc.snap.md`: legacy-decorators count
  goes from 8/96 to 9/96 (the new accessor fixture passes; the format is
  passing/total, so my prior hand-edit had it backwards by one).
- `coverage/snapshots/semantic_typescript.snap`: the
  `esNextWeakRefs_IterableWeakMap.ts` case exercises legacy decorators
  with accessors. The transformer now serializes the actual type
  instead of falling through to `Object`, which shifts reference IDs
  for `_decorateMetadata` and removes `Object` from the unresolved
  reference list at that test site.
- `coverage/snapshots/semantic_misc.snap`: same shape — `oxc-1288.ts`
  now emits a `String` reference where previously none existed.

Co-Authored-By: Claude <noreply@anthropic.com>
…itive types

Adds fixture cases for the runtime-guard path that the previous fixture
didn't exercise:

- `string[]` → `Array` (statically resolved)
- `Promise<string>` → runtime guard around the global `Promise` constructor
- `Addr` (same-file class) → runtime guard around the class binding

Also aligns the `dec` signature between `input.ts` and `exec.ts` to
`function dec() {}` (the parameter list isn't relevant to the bug and
having unused params triggered a TS unused-parameter hint locally).

Co-Authored-By: Claude <noreply@anthropic.com>
…erence flag mismatch

The previous expansion added `Promise<string>` and `Addr` cases that
exercise the runtime-guard path. That path has a pre-existing
metadata-reference-flag inconsistency (the transformer's post-transform
scoping marks the runtime-emitted identifier as `Type|Read` while the
rebuilt analysis sees `Read`) that's already covered by the existing
`oxc/metadata/bound-type-reference` fixture. Adding the same pattern in
this PR's fixture muddies the signal — the new fixture is meant to
demonstrate that this PR's bug is fixed, not to re-test the unrelated
reference-flag issue.

Keep `string[]` → `Array` (statically resolved, no type references) and
drop `Promise<string>` and `Addr`. The runtime-guard path remains
covered by `bound-type-reference`.

Co-Authored-By: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant