fix(transformer/legacy-decorator): preserve accessor type annotation for emitDecoratorMetadata#21966
Draft
fix(transformer/legacy-decorator): preserve accessor type annotation for emitDecoratorMetadata#21966
Conversation
Member
Author
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
|
…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>
bea18eb to
fc921b8
Compare
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Fixes bug 7 of #21922.
The bug
@dec accessor x: Talways emitteddesign:type = Objectregardless ofT:Reflect.getMetadata("design:type", entity, "name")returnedObjectinstead ofString. Frameworks readingdesign:type(NestJS Swagger, class-validator, TypeORM column metadata) silently degraded for every typed accessor field.Cause
lower_accessor_properties(inenter_class) rewritesinto 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:
return_typeisNonebecause lowering discarded it, soserialize_type_annotationcorrectly fell through toObjectfor what it sees as an untyped getter.The metadata pass's
enter_accessor_propertydispatch exists, but it never fires because theAccessorPropertyno longer exists by the time traversal reaches the class body.Fix
Stop dropping the type annotation. In
lower_accessor_properties:Thread
type_annotationthroughcreate_accessor_methodto the getter (setter passesNone) and attach it as the synth function'sreturn_type. The existing getter-metadata path then serializes it correctly.The lowered AST is now self-consistent — the synth getter for
accessor x: Treads as a getter that returnsT, which is what it semantically is.Out of scope (architectural limit)
type Uuid = string; @dec accessor x: Uuidstill emitsObject, same as for non-accessor properties. Resolving type-alias references to their primitive constituent requires a name-based alias-resolution pass (the same one thattsc's checker provides viagetTypeReferenceSerializationKind). 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
oxc/metadata/accessorcovers primitives, untyped (correctly staysObject),string[]→Array, andstatic accessorcargo test -p oxc_transformer— 31 + 5 passedcargo 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