Repository navigation
fix(transpiler): treat emitDecoratorMetadata as implying legacy decorators - #27527
Jarred-Sumner merged 1 commit into
Conversation
|
Updated 1:58 AM PT - Feb 28th, 2026
❌ Your commit
🧪 To try this PR locally: bunx bun-pr 27527That installs a local version of the PR into your bun-27527 --bun |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (3)
WalkthroughAdjusts decorator-selection logic so legacy/experimental decorator semantics are used when TypeScript's emitDecoratorMetadata is enabled; adds explanatory comments and adds regression tests covering both emitDecoratorMetadata and no-decorator-flag scenarios. Changes
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
…ators When `emitDecoratorMetadata: true` is set in tsconfig.json without `experimentalDecorators: true`, Bun now correctly uses legacy decorator semantics instead of TC39 standard decorator lowering. `emitDecoratorMetadata` is a TypeScript feature that only makes sense with the legacy decorator system (reflect-metadata). TC39 standard decorators have their own metadata mechanism. This fixes a regression introduced in ce715b5 where frameworks like NestJS, TypeORM, and Angular would crash with `descriptor.value` undefined when their tsconfig did not explicitly set `experimentalDecorators: true`. Closes #27526 Co-Authored-By: Claude <noreply@anthropic.com>
4a5c20b to
00553cd
Compare
There was a problem hiding this comment.
LGTM — straightforward one-line condition fix applied consistently in both code paths, with a good regression test.
Extended reasoning...
Overview
This PR fixes a regression where emitDecoratorMetadata: true in tsconfig.json (without experimentalDecorators: true) caused Bun to use TC39 standard decorator lowering instead of legacy decorators. The fix adds emit_decorator_metadata to the condition that determines whether to use standard vs. legacy decorators, applied identically in src/bundler/ParseTask.zig and src/transpiler.zig. A regression test validates both legacy and standard decorator paths.
Security risks
None. This is a transpiler semantics fix that only affects how decorators are lowered — no auth, crypto, permissions, or external I/O changes.
Level of scrutiny
This is production-critical transpiler code, but the change is minimal and mechanical: a single boolean condition extended with an or clause, applied identically in two locations. The logic is sound — emitDecoratorMetadata is a TypeScript-only feature tied to reflect-metadata and legacy decorators; TC39 standard decorators have their own metadata mechanism. The change correctly treats emitDecoratorMetadata as implying legacy decorator intent.
Other factors
- The regression test covers both the fixed case (legacy decorators with
emitDecoratorMetadata) and the unaffected case (TC39 standard decorators when neither flag is set). - CodeRabbit found no issues. No bugs were found by the bug hunting system.
- No outstanding reviewer comments or prior reviews to address.
- The PR is well-scoped and self-contained.
…ators (oven-sh#27527) ## Summary - When `emitDecoratorMetadata: true` is set in tsconfig.json without `experimentalDecorators: true`, Bun now correctly uses legacy decorator semantics instead of TC39 standard decorator lowering - `emitDecoratorMetadata` only makes sense with TypeScript's legacy decorator system (`reflect-metadata`), so its presence implies the user expects legacy decorator behavior - This fixes a regression from ce715b5 where NestJS, TypeORM, Angular, and other legacy-decorator frameworks would crash with `descriptor.value` undefined Closes oven-sh#27526 ## Test plan - [x] Added regression test `test/regression/issue/27526.test.ts` with two cases: - Legacy decorators work when `emitDecoratorMetadata: true` but `experimentalDecorators` is absent - TC39 standard decorators still work when neither option is set - [x] Verified test fails with system Bun (`USE_SYSTEM_BUN=1`) and passes with debug build (`bun bd test`) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Bot <claude-bot@bun.sh> Co-authored-by: Claude <noreply@anthropic.com>
…ators (oven-sh#27527) ## Summary - When `emitDecoratorMetadata: true` is set in tsconfig.json without `experimentalDecorators: true`, Bun now correctly uses legacy decorator semantics instead of TC39 standard decorator lowering - `emitDecoratorMetadata` only makes sense with TypeScript's legacy decorator system (`reflect-metadata`), so its presence implies the user expects legacy decorator behavior - This fixes a regression from ce715b5 where NestJS, TypeORM, Angular, and other legacy-decorator frameworks would crash with `descriptor.value` undefined Closes oven-sh#27526 ## Test plan - [x] Added regression test `test/regression/issue/27526.test.ts` with two cases: - Legacy decorators work when `emitDecoratorMetadata: true` but `experimentalDecorators` is absent - TC39 standard decorators still work when neither option is set - [x] Verified test fails with system Bun (`USE_SYSTEM_BUN=1`) and passes with debug build (`bun bd test`) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Bot <claude-bot@bun.sh> Co-authored-by: Claude <noreply@anthropic.com>
Summary
emitDecoratorMetadata: trueis set in tsconfig.json withoutexperimentalDecorators: true, Bun now correctly uses legacy decorator semantics instead of TC39 standard decorator loweringemitDecoratorMetadataonly makes sense with TypeScript's legacy decorator system (reflect-metadata), so its presence implies the user expects legacy decorator behaviordescriptor.valueundefinedCloses #27526
Test plan
test/regression/issue/27526.test.tswith two cases:emitDecoratorMetadata: truebutexperimentalDecoratorsis absentUSE_SYSTEM_BUN=1) and passes with debug build (bun bd test)🤖 Generated with Claude Code