Repository navigation
resolver: merge experimentalDecorators across tsconfig extends chain #30478
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
robobun
wants to merge
8
commits into
main
from
farm/31c67eff/merge-experimental-decorators-across-extends
Closed
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
ae5ee3b
resolver: merge experimentalDecorators across tsconfig extends chain
robobun 7c8ce85
[autofix.ci] apply automated fixes
autofix-ci[bot] 2da0a85
tsconfig: use child-wins (not OR) semantics for decorator flag merge
robobun f327e99
review: concurrent tests + runEnvLoader tsconfig fallback
robobun cebfafe
review: assert empty stderr + stdout-before-exitCode
robobun f578136
test: conditional stderr assertion (Windows prints to stderr, breaks …
robobun 0c2a38e
test: verify emitDecoratorMetadata via runtime Reflect shim
robobun c00899a
ci: retrigger — windows test-http-close timeout + flaky unrelated tes…
robobun File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,213 @@ | ||
| import { expect, test } from "bun:test"; | ||
| import { bunEnv, bunExe, tempDir } from "harness"; | ||
|
|
||
| // https://github.com/oven-sh/bun/issues/30477 | ||
| // experimentalDecorators was silently dropped across tsconfig extends chains. | ||
|
|
||
| // The probe decorator distinguishes legacy vs stage-3 emit at runtime: | ||
| // legacy decorators pass exactly one argument (the target class); | ||
| // stage-3 decorators pass two (value + context). | ||
| const PROBE_SOURCE = ` | ||
| function probe(...args: unknown[]) { | ||
| if (args.length === 1) { | ||
| console.log("legacy"); | ||
| } else { | ||
| console.log("stage-3"); | ||
| } | ||
| } | ||
|
|
||
| @probe | ||
| class Foo {} | ||
|
|
||
| console.log("OK"); | ||
| `; | ||
|
|
||
| test.concurrent("experimentalDecorators: true is preserved through an extends chain", async () => { | ||
| using dir = tempDir("bun-30477-extends", { | ||
| "base-tsconfig.json": JSON.stringify({ | ||
| compilerOptions: { target: "esnext" }, | ||
| }), | ||
| "tsconfig.json": JSON.stringify({ | ||
| extends: "./base-tsconfig.json", | ||
| compilerOptions: { module: "esnext", experimentalDecorators: true }, | ||
| }), | ||
| "index.ts": PROBE_SOURCE, | ||
| }); | ||
|
|
||
| await using proc = Bun.spawn({ | ||
| cmd: [bunExe(), "index.ts"], | ||
| env: bunEnv, | ||
| cwd: String(dir), | ||
| stderr: "pipe", | ||
| }); | ||
|
|
||
| const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); | ||
|
|
||
| expect(stdout).toBe("legacy\nOK\n"); | ||
| if (exitCode !== 0) { | ||
| expect(stderr).toBe(""); | ||
| } | ||
| expect(exitCode).toBe(0); | ||
|
robobun marked this conversation as resolved.
coderabbitai[bot] marked this conversation as resolved.
|
||
| }); | ||
|
|
||
| test.concurrent("experimentalDecorators inherited from the base tsconfig still wins", async () => { | ||
| using dir = tempDir("bun-30477-base", { | ||
| "base-tsconfig.json": JSON.stringify({ | ||
| compilerOptions: { target: "esnext", experimentalDecorators: true }, | ||
| }), | ||
| "tsconfig.json": JSON.stringify({ | ||
| extends: "./base-tsconfig.json", | ||
| compilerOptions: { module: "esnext" }, | ||
| }), | ||
| "index.ts": PROBE_SOURCE, | ||
| }); | ||
|
|
||
| await using proc = Bun.spawn({ | ||
| cmd: [bunExe(), "index.ts"], | ||
| env: bunEnv, | ||
| cwd: String(dir), | ||
| stderr: "pipe", | ||
| }); | ||
|
|
||
| const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); | ||
|
|
||
| expect(stdout).toBe("legacy\nOK\n"); | ||
| if (exitCode !== 0) { | ||
| expect(stderr).toBe(""); | ||
| } | ||
| expect(exitCode).toBe(0); | ||
| }); | ||
|
|
||
| // The following two cases pin down TypeScript's per-key override semantics for | ||
| // `extends`: a child's explicit value wins over the parent's, even when the | ||
| // child's value is `false`. Without this, `or`-merging made `true` sticky — | ||
| // a base config could force legacy decorators on every child that extended it. | ||
| test.concurrent("child experimentalDecorators: false overrides parent true (disables legacy)", async () => { | ||
| using dir = tempDir("bun-30477-child-false-exp", { | ||
| "base-tsconfig.json": JSON.stringify({ | ||
| compilerOptions: { target: "esnext", experimentalDecorators: true }, | ||
| }), | ||
| "tsconfig.json": JSON.stringify({ | ||
| extends: "./base-tsconfig.json", | ||
| compilerOptions: { module: "esnext", experimentalDecorators: false }, | ||
| }), | ||
| "index.ts": PROBE_SOURCE, | ||
| }); | ||
|
|
||
| await using proc = Bun.spawn({ | ||
| cmd: [bunExe(), "index.ts"], | ||
| env: bunEnv, | ||
| cwd: String(dir), | ||
| stderr: "pipe", | ||
| }); | ||
|
|
||
| const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); | ||
|
|
||
| // Child explicitly opts out of legacy decorators — stage-3 should win. | ||
| expect(stdout).toBe("stage-3\nOK\n"); | ||
| if (exitCode !== 0) { | ||
| expect(stderr).toBe(""); | ||
| } | ||
| expect(exitCode).toBe(0); | ||
| }); | ||
|
|
||
| test.concurrent("child emitDecoratorMetadata: false overrides parent true", async () => { | ||
| // When emitDecoratorMetadata is true, Bun imports reflect-metadata and | ||
| // emits __metadata() calls — the decorator receives argument-type info | ||
| // reflected into `design:paramtypes`. A child that sets the flag back to | ||
| // false must disable that emission, leaving the legacy decorator call | ||
| // with no reflect-metadata lookup at all. | ||
| // | ||
| // We verify via runtime observation (is Reflect.getMetadata reachable?) | ||
| // rather than scanning bundler output — the test is then insensitive to | ||
| // how the bundled runtime helpers happen to be named. | ||
| const META_SOURCE = ` | ||
| // Report whether the decorated class saw design:paramtypes metadata. | ||
| // If emitDecoratorMetadata is on, the decorator calls below would have | ||
| // invoked Reflect.metadata("design:paramtypes", [Number]) and we'd read | ||
| // it back via Reflect.getMetadata. | ||
| (globalThis as any).Reflect = (globalThis as any).Reflect ?? {}; | ||
| const metaMap = new WeakMap<object, any>(); | ||
| (globalThis as any).Reflect.metadata = (k: string, v: unknown) => (t: object, p: string) => { | ||
| let entry = metaMap.get(t); | ||
| if (!entry) metaMap.set(t, entry = {}); | ||
| entry[p] = entry[p] ?? {}; | ||
| entry[p][k] = v; | ||
| }; | ||
|
|
||
| function probe(target: unknown, key: unknown) {} | ||
|
|
||
| class Foo { | ||
| @probe | ||
| foo(a: number) {} | ||
| } | ||
|
|
||
| // If the child's emitDecoratorMetadata: false was respected, our | ||
| // Reflect.metadata shim was never called and the WeakMap is empty. | ||
| console.log(metaMap.has(Foo.prototype) ? "HAS_METADATA" : "NO_METADATA"); | ||
| `; | ||
| using dir = tempDir("bun-30477-child-false-meta", { | ||
| "base-tsconfig.json": JSON.stringify({ | ||
| compilerOptions: { | ||
| target: "esnext", | ||
| experimentalDecorators: true, | ||
| emitDecoratorMetadata: true, | ||
| }, | ||
| }), | ||
| "tsconfig.json": JSON.stringify({ | ||
| extends: "./base-tsconfig.json", | ||
| compilerOptions: { module: "esnext", emitDecoratorMetadata: false }, | ||
| }), | ||
| "index.ts": META_SOURCE, | ||
| }); | ||
|
|
||
| await using proc = Bun.spawn({ | ||
| cmd: [bunExe(), "./index.ts"], | ||
| env: bunEnv, | ||
| cwd: String(dir), | ||
| stderr: "pipe", | ||
| }); | ||
|
|
||
| const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); | ||
|
|
||
| // Child opted out of decorator metadata — the runtime shim must not have | ||
| // been called, so the map stays empty. | ||
| expect(stdout).toBe("NO_METADATA\n"); | ||
| if (exitCode !== 0) { | ||
| expect(stderr).toBe(""); | ||
| } | ||
| expect(exitCode).toBe(0); | ||
| }); | ||
|
|
||
| test.concurrent("--tsconfig-override picks up experimentalDecorators via extends", async () => { | ||
| using dir = tempDir("bun-30477-override", { | ||
| "tsconfig.json": JSON.stringify({ | ||
| compilerOptions: { target: "esnext" }, | ||
| }), | ||
| "tsconfig.bun.json": JSON.stringify({ | ||
| extends: "./tsconfig.json", | ||
| compilerOptions: { | ||
| module: "esnext", | ||
| experimentalDecorators: true, | ||
| }, | ||
| }), | ||
| "index.ts": PROBE_SOURCE, | ||
| }); | ||
|
|
||
| await using proc = Bun.spawn({ | ||
| cmd: [bunExe(), "--tsconfig-override", "./tsconfig.bun.json", "./index.ts"], | ||
| env: bunEnv, | ||
| cwd: String(dir), | ||
| stderr: "pipe", | ||
| }); | ||
|
|
||
| const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); | ||
|
|
||
| expect(stdout).toBe("legacy\nOK\n"); | ||
| // The bogus "Internal error: directory mismatch" warning from the | ||
| // override-fd path must not appear — the resolver now passes an | ||
| // invalid dirname_fd when the override path isn't a child of the | ||
| // directory being iterated. | ||
| expect(stderr).not.toContain("directory mismatch"); | ||
| expect(exitCode).toBe(0); | ||
| }); | ||
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.