Conversation
WalkthroughThreads a new strict_null_checks flag through TSConfig/resolver, transpiler, bundler, and parser paths; updates TypeScript union merging to respect strict null checks; and adds regression tests validating decorator metadata emission under various strictNullChecks/strict settings. Changes
Suggested reviewers
Pre-merge checks✅ Passed checks (2 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/bundler/bundle_v2.zig (1)
1369-1375: Don’t hardcode strict_null_checks=false for ParseTask2; thread the real setting.This breaks parity for generated/SSR files vs normal parse tasks and can regress the union→Object behavior under strictNullChecks. Use the target’s option instead.
Apply:
- .emit_decorator_metadata = false, // TODO - .strict_null_checks = false, // TODO + .emit_decorator_metadata = false, // TODO (follow up if needed) + .strict_null_checks = this.transpilerForTarget(known_target).options.strict_null_checks,
♻️ Duplicate comments (4)
test/regression/issue/decorator-metadata-unions.test.ts (4)
137-144: Useawait usingfor automatic resource cleanup.Per past review comments, this spawn should use
await usingfor proper cleanup.Apply this diff:
- const tscProc = Bun.spawn({ + await using tscProc = Bun.spawn({ cmd: [bunExe(), "x", "tsc"], cwd: String(dir), env: bunEnv, stdout: "pipe", stderr: "pipe", }); - await tscProc.exited; -
147-154: Use Bun.build API instead of spawning a build process.Per past review comments, use the Bun.build API and await both the build and tsc processes with Promise.all for better parallelization and resource management.
Example refactor:
const buildPromise = Bun.build({ entrypoints: [`${dir}/test.ts`], outdir: String(dir), naming: "[name]-bun.js", }); await Promise.all([buildPromise, tscProc.exited]);
174-174: Usetest.concurrentfor better test performance.Per past review comments, this test can run concurrently with others.
Apply this diff:
-test("decorator metadata with non-union types emits actual type", async () => { +test.concurrent("decorator metadata with non-union types emits actual type", async () => {
213-220: Useawait usingfor automatic resource cleanup.Per past review comments, the install process should use
await using.Apply this diff:
- const installProc = Bun.spawn({ + await using installProc = Bun.spawn({ cmd: [bunExe(), "install"], cwd: String(dir), env: bunEnv, stdout: "pipe", stderr: "pipe", }); - await installProc.exited;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (13)
src/ast/TypeScript.zig(1 hunks)src/ast/skipTypescript.zig(1 hunks)src/bun.js/ModuleLoader.zig(3 hunks)src/bun.js/RuntimeTranspilerStore.zig(1 hunks)src/bundler/ParseTask.zig(3 hunks)src/bundler/bundle_v2.zig(1 hunks)src/options.zig(1 hunks)src/resolver/resolver.zig(3 hunks)src/resolver/tsconfig_json.zig(2 hunks)src/runtime.zig(1 hunks)src/transpiler.zig(4 hunks)test/regression/issue/decorator-metadata-strict-null-checks.test.ts(1 hunks)test/regression/issue/decorator-metadata-unions.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (11)
**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)
**/*.zig: Declare the extern C symbol in Zig and export a Zig-friendly alias for use
Wrap the Bun____toJS extern in a Zig method that takes a JSGlobalObject and returns JSC.JSValue
Files:
src/bun.js/RuntimeTranspilerStore.zigsrc/ast/skipTypescript.zigsrc/options.zigsrc/runtime.zigsrc/ast/TypeScript.zigsrc/bundler/bundle_v2.zigsrc/bundler/ParseTask.zigsrc/resolver/tsconfig_json.zigsrc/bun.js/ModuleLoader.zigsrc/resolver/resolver.zigsrc/transpiler.zig
src/bun.js/**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/zig-javascriptcore-classes.mdc)
src/bun.js/**/*.zig: In Zig binding structs, expose generated bindings via pub const js = JSC.Codegen.JS and re-export toJS/fromJS/fromJSDirect
Constructors and prototype methods should return bun.JSError!JSC.JSValue to integrate Zig error handling with JS exceptions
Use parameter name globalObject (not ctx) and accept (*JSC.JSGlobalObject, *JSC.CallFrame) in binding methods/constructors
Implement getters as get(this, globalObject) returning JSC.JSValue and matching the .classes.ts interface
Provide deinit() for resource cleanup and finalize() that calls deinit(); use bun.destroy(this) or appropriate destroy pattern
Access JS call data via CallFrame (argument(i), argumentCount(), thisValue()) and throw errors with globalObject.throw(...)
For properties marked cache: true, use the generated Zig accessors (NameSetCached/GetCached) to work with GC-owned values
In finalize() for objects holding JS references, release them using .deref() before destroy
Files:
src/bun.js/RuntimeTranspilerStore.zigsrc/bun.js/ModuleLoader.zig
src/**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)
When adding debug logs in Zig, create a scoped logger and log via Bun APIs:
const log = bun.Output.scoped(.${SCOPE}, .hidden);thenlog("...", .{})
src/**/*.zig: Use Zig private fields with the # prefix for encapsulation (e.g., struct { #foo: u32 })
Prefer Decl literals for initialization (e.g., const decl: Decl = .{ .binding = 0, .value = 0 };)
Place @import statements at the bottom of the file (formatter will handle ordering)
src/**/*.zig: In Zig code, manage memory carefully: use appropriate allocators and defer for cleanup
Cache JavaScriptCore class structures in ZigGlobalObject when adding new classes
Files:
src/bun.js/RuntimeTranspilerStore.zigsrc/ast/skipTypescript.zigsrc/options.zigsrc/runtime.zigsrc/ast/TypeScript.zigsrc/bundler/bundle_v2.zigsrc/bundler/ParseTask.zigsrc/resolver/tsconfig_json.zigsrc/bun.js/ModuleLoader.zigsrc/resolver/resolver.zigsrc/transpiler.zig
test/**
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Place all tests under the test/ directory
Files:
test/regression/issue/decorator-metadata-unions.test.tstest/regression/issue/decorator-metadata-strict-null-checks.test.ts
test/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
test/**/*.{js,ts}: Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test
Prefer data-driven tests (e.g., test.each) to reduce boilerplate
Use shared utilities from test/harness.ts where applicable
Files:
test/regression/issue/decorator-metadata-unions.test.tstest/regression/issue/decorator-metadata-strict-null-checks.test.ts
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}
📄 CodeRabbit inference engine (test/CLAUDE.md)
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Usebun:testfor files ending with*.test.{ts,js,jsx,tsx,mjs,cjs}
Prefer concurrent tests (test.concurrent/describe.concurrent) over sequential when feasible
Organize tests withdescribeblocks to group related tests
Use utilities likedescribe.each,toMatchSnapshot, and lifecycle hooks (beforeAll,beforeEach,afterEach) and track resources for cleanup
Files:
test/regression/issue/decorator-metadata-unions.test.tstest/regression/issue/decorator-metadata-strict-null-checks.test.ts
test/**/*.{ts,tsx,js,jsx,mjs,cjs}
📄 CodeRabbit inference engine (test/CLAUDE.md)
For large/repetitive strings, use
Buffer.alloc(count, fill).toString()instead of"A".repeat(count)
Files:
test/regression/issue/decorator-metadata-unions.test.tstest/regression/issue/decorator-metadata-strict-null-checks.test.ts
test/regression/issue/*.test.ts
📄 CodeRabbit inference engine (test/CLAUDE.md)
Place regression tests for specific issues in
/test/regression/issue/${issueNumber}.test.tsPlace tests for specific numbered GitHub issues in test/regression/issue/${issueNumber}.test.ts and ensure the issue number is real
Files:
test/regression/issue/decorator-metadata-unions.test.tstest/regression/issue/decorator-metadata-strict-null-checks.test.ts
test/regression/**
📄 CodeRabbit inference engine (test/CLAUDE.md)
Do not place tests without an issue number in the regression directory
Files:
test/regression/issue/decorator-metadata-unions.test.tstest/regression/issue/decorator-metadata-strict-null-checks.test.ts
test/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
test/**/*.test.{ts,tsx}: Test files must live under test/ and end with .test.ts or .test.tsx
In tests, always use port: 0; do not hardcode ports or invent random port functions
Prefer snapshot assertions and use normalizeBunSnapshot for snapshot output in tests
Never write tests that assert absence of crashes (e.g., no "panic" or "uncaught exception") in output
Use tempDir from "harness" for temporary directories; do not use tmpdirSync or fs.mkdtempSync in tests
When spawning processes in tests, assert on stdout before asserting exitCode
Do not use setTimeout in tests; await conditions instead to avoid flakiness
Files:
test/regression/issue/decorator-metadata-unions.test.tstest/regression/issue/decorator-metadata-strict-null-checks.test.ts
test/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid shell commands in tests (e.g., find, grep); use Bun's Glob and built-in tools
Files:
test/regression/issue/decorator-metadata-unions.test.tstest/regression/issue/decorator-metadata-strict-null-checks.test.ts
🧠 Learnings (62)
📓 Common learnings
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23169
File: src/bun.js/bindings/BunIDLConvert.h:29-42
Timestamp: 2025-10-01T21:49:27.862Z
Learning: In Bun's IDL bindings (src/bun.js/bindings/BunIDLConvert.h), IDLStrictNull intentionally treats both undefined and null as null (using isUndefinedOrNull()), matching WebKit's IDLNull & IDLNullable behavior. This is the correct implementation and should not be changed to only accept null.
Learnt from: cirospaciari
Repo: oven-sh/bun PR: 22946
File: test/js/sql/sql.test.ts:195-202
Timestamp: 2025-09-25T22:07:13.851Z
Learning: PR oven-sh/bun#22946: JSON/JSONB result parsing updates (e.g., returning parsed arrays instead of legacy strings) are out of scope for this PR; tests keep current expectations with a TODO. Handle parsing fixes in a separate PR.
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/bindings/generated_classes_list.zig : Update src/bun.js/bindings/generated_classes_list.zig to include new classes
Learnt from: cirospaciari
Repo: oven-sh/bun PR: 22946
File: src/js/internal/sql/postgres.ts:188-198
Timestamp: 2025-09-25T01:02:43.263Z
Learning: In the PostgreSQL array implementation for Bun.SQL, when getPostgresArrayType returns null for unknown numeric OIDs, throwing an error is not the desired behavior. The user prefers a different approach for handling unknown array type OIDs.
📚 Learning: 2025-10-18T20:50:47.750Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: src/bun.js/telemetry.zig:366-373
Timestamp: 2025-10-18T20:50:47.750Z
Learning: In Bun's Zig codebase (src/bun.js/bindings/JSValue.zig), the JSValue enum uses `.null` (not `.js_null`) for JavaScript's null value. Only `js_undefined` has the `js_` prefix to avoid collision with Zig's built-in `undefined` keyword. The correct enum fields are: `js_undefined`, `null`, `true`, `false`, and `zero`.
Applied to files:
src/bun.js/RuntimeTranspilerStore.zigsrc/options.zigsrc/ast/TypeScript.zigsrc/bundler/bundle_v2.zigsrc/bundler/ParseTask.zigsrc/resolver/tsconfig_json.zigsrc/bun.js/ModuleLoader.zigsrc/resolver/resolver.zigsrc/transpiler.zig
📚 Learning: 2025-08-30T00:13:36.815Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/bindings/generated_classes_list.zig : Update src/bun.js/bindings/generated_classes_list.zig to include new classes
Applied to files:
src/bun.js/RuntimeTranspilerStore.zigsrc/options.zigsrc/ast/TypeScript.zigsrc/bun.js/ModuleLoader.zig
📚 Learning: 2025-08-30T00:13:36.815Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/**/*.zig : In Zig binding structs, expose generated bindings via pub const js = JSC.Codegen.JS<ClassName> and re-export toJS/fromJS/fromJSDirect
Applied to files:
src/bun.js/RuntimeTranspilerStore.zigsrc/bun.js/ModuleLoader.zigsrc/transpiler.zig
📚 Learning: 2025-08-30T00:11:57.076Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/registering-bun-modules.mdc:0-0
Timestamp: 2025-08-30T00:11:57.076Z
Learning: Applies to src/{**/js_*.zig,bun.js/api/**/*.zig} : Use bun.JSError!JSValue for proper error propagation in JS-exposed Zig functions
Applied to files:
src/bun.js/RuntimeTranspilerStore.zigsrc/bun.js/ModuleLoader.zigsrc/transpiler.zig
📚 Learning: 2025-10-01T21:49:27.862Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23169
File: src/bun.js/bindings/BunIDLConvert.h:29-42
Timestamp: 2025-10-01T21:49:27.862Z
Learning: In Bun's IDL bindings (src/bun.js/bindings/BunIDLConvert.h), IDLStrictNull intentionally treats both undefined and null as null (using isUndefinedOrNull()), matching WebKit's IDLNull & IDLNullable behavior. This is the correct implementation and should not be changed to only accept null.
Applied to files:
src/bun.js/RuntimeTranspilerStore.zigsrc/ast/TypeScript.zigsrc/bun.js/ModuleLoader.zig
📚 Learning: 2025-09-08T00:41:12.052Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-09-08T00:41:12.052Z
Learning: Applies to src/bun.js/bindings/v8/src/napi/napi.zig : Add new V8 API method mangled symbols to the V8API struct in src/napi/napi.zig for both GCC/Clang and MSVC
Applied to files:
src/bun.js/RuntimeTranspilerStore.zigsrc/bun.js/ModuleLoader.zigsrc/transpiler.zig
📚 Learning: 2025-09-02T18:25:27.976Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 22227
File: src/allocators/allocation_scope.zig:284-314
Timestamp: 2025-09-02T18:25:27.976Z
Learning: In bun's custom Zig implementation, the `#` prefix for private fields is valid syntax and should not be flagged as invalid. The syntax `#fieldname` creates private fields that cannot be accessed from outside the defining struct, and usage like `self.#fieldname` is correct within the same struct. This applies to fields like `#parent`, `#state`, `#allocator`, `#trace`, etc. throughout the codebase.
Applied to files:
src/bun.js/RuntimeTranspilerStore.zigsrc/options.zigsrc/bundler/bundle_v2.zig
📚 Learning: 2025-09-02T17:14:46.924Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 22227
File: src/safety/alloc.zig:93-95
Timestamp: 2025-09-02T17:14:46.924Z
Learning: In bun's Zig codebase, they use a custom extension of Zig that supports private field syntax with the `#` prefix (e.g., `#allocator`, `#trace`). This is not standard Zig syntax but is valid in their custom implementation. Fields prefixed with `#` are private fields that cannot be accessed from outside the defining struct.
Applied to files:
src/bun.js/RuntimeTranspilerStore.zigsrc/options.zig
📚 Learning: 2025-10-25T22:53:31.261Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/CLAUDE.md:0-0
Timestamp: 2025-10-25T22:53:31.261Z
Learning: Applies to src/**/*.zig : Prefer Decl literals for initialization (e.g., const decl: Decl = .{ .binding = 0, .value = 0 };)
Applied to files:
src/bun.js/RuntimeTranspilerStore.zigsrc/options.zigsrc/bundler/bundle_v2.zigsrc/bundler/ParseTask.zigsrc/resolver/tsconfig_json.zigsrc/transpiler.zig
📚 Learning: 2025-11-03T20:43:06.996Z
Learnt from: pfgithub
Repo: oven-sh/bun PR: 24273
File: src/bun.js/test/snapshot.zig:19-19
Timestamp: 2025-11-03T20:43:06.996Z
Learning: In Bun's Zig codebase, when storing JSValue objects in collections like ArrayList, use `jsc.Strong.Optional` (not raw JSValue). When adding values, wrap them with `jsc.Strong.Optional.create(value, globalThis)`. In cleanup code, iterate the collection calling `.deinit()` on each Strong.Optional item before calling `.deinit()` on the ArrayList itself. This pattern automatically handles GC protection. See examples in src/bun.js/test/ScopeFunctions.zig and src/bun.js/node/node_cluster_binding.zig.
Applied to files:
src/bun.js/RuntimeTranspilerStore.zigsrc/options.zig
📚 Learning: 2025-10-24T10:43:09.398Z
Learnt from: fmguerreiro
Repo: oven-sh/bun PR: 23774
File: src/install/PackageManager/updatePackageJSONAndInstall.zig:548-548
Timestamp: 2025-10-24T10:43:09.398Z
Learning: In Bun's Zig codebase, the `as(usize, intCast(...))` cast pattern triggers a Zig compiler bug that causes compilation to hang indefinitely when used in complex control flow contexts (loops + short-circuit operators + optional unwrapping). Avoid this pattern and use simpler alternatives like just `intCast(...)` if type casting is necessary.
Applied to files:
src/ast/skipTypescript.zig
📚 Learning: 2025-10-04T09:52:49.414Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-10-04T09:52:49.414Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Use process.platform and process.arch for platform detection (rely on inlining/dead-code elimination)
Applied to files:
src/ast/skipTypescript.zigtest/regression/issue/decorator-metadata-unions.test.tssrc/resolver/resolver.zig
📚 Learning: 2025-10-16T02:17:35.237Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23710
File: src/analytics.zig:15-21
Timestamp: 2025-10-16T02:17:35.237Z
Learning: In src/analytics.zig and similar files using bun.EnvVar boolean environment variables: the new EnvVar API for boolean flags (e.g., bun.EnvVar.do_not_track.get(), bun.EnvVar.ci.get()) is designed to parse and return boolean values from environment variables, not just check for their presence. This is an intentional design change from the previous presence-based checks using bun.getenvZ().
Applied to files:
src/options.zigsrc/runtime.zigsrc/bundler/bundle_v2.zigsrc/resolver/tsconfig_json.zig
📚 Learning: 2025-09-06T03:37:41.154Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 22229
File: src/bundler/LinkerGraph.zig:0-0
Timestamp: 2025-09-06T03:37:41.154Z
Learning: In Bun's codebase, when checking import record source indices in src/bundler/LinkerGraph.zig, prefer using `if (import_index >= self.import_records.len)` bounds checking over `isValid()` checks, as the bounds check is more robust and `isValid()` is a strict subset of this condition.
Applied to files:
src/options.zigsrc/bundler/bundle_v2.zig
📚 Learning: 2025-10-16T21:24:52.779Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23710
File: src/crash_handler.zig:1415-1423
Timestamp: 2025-10-16T21:24:52.779Z
Learning: When a boolean EnvVar in src/envvars.zig is defined with a default value (e.g., `.default = false`), the `get()` method returns `bool` instead of `?bool`. This means you cannot distinguish between "environment variable not set" and "environment variable explicitly set to the default value". For opt-out scenarios where detection of explicit setting is needed (like `BUN_ENABLE_CRASH_REPORTING` on platforms where crash reporting defaults to enabled), either: (1) don't provide a default value so `get()` returns `?bool`, or (2) use the returned boolean directly instead of only checking if it's true.
Applied to files:
src/options.zig
📚 Learning: 2025-10-25T22:53:31.261Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/CLAUDE.md:0-0
Timestamp: 2025-10-25T22:53:31.261Z
Learning: Applies to src/**/*.zig : Use Zig private fields with the # prefix for encapsulation (e.g., struct { #foo: u32 })
Applied to files:
src/options.zigsrc/resolver/tsconfig_json.zigsrc/transpiler.zig
📚 Learning: 2025-10-15T22:03:50.832Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23710
File: src/envvars.zig:135-144
Timestamp: 2025-10-15T22:03:50.832Z
Learning: In src/envvars.zig, the boolean feature flag cache uses a single atomic enum and should remain monotonic. Only the string cache (which uses two atomics: ptr and len) requires acquire/release ordering to prevent torn reads.
Applied to files:
src/runtime.zig
📚 Learning: 2025-08-30T00:11:57.076Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/registering-bun-modules.mdc:0-0
Timestamp: 2025-08-30T00:11:57.076Z
Learning: Implement the core feature in Zig under its own directory within src/<feature>/
Applied to files:
src/runtime.zig
📚 Learning: 2025-10-26T05:04:50.692Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-26T05:04:50.692Z
Learning: Applies to test/**/*.test.{ts,tsx} : Prefer snapshot assertions and use normalizeBunSnapshot for snapshot output in tests
Applied to files:
test/regression/issue/decorator-metadata-unions.test.tstest/regression/issue/decorator-metadata-strict-null-checks.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/html.test.ts : html.test.ts should contain tests relating to HTML files themselves
Applied to files:
test/regression/issue/decorator-metadata-unions.test.tstest/regression/issue/decorator-metadata-strict-null-checks.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/bundle.test.ts : bundle.test.ts should contain DevServer-specific bundling tests
Applied to files:
test/regression/issue/decorator-metadata-unions.test.tstest/regression/issue/decorator-metadata-strict-null-checks.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/plugins.test.ts : plugins.test.ts should contain plugin-related development-mode tests
Applied to files:
test/regression/issue/decorator-metadata-unions.test.tstest/regression/issue/decorator-metadata-strict-null-checks.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/{dev/*.test.ts,dev-and-prod.ts} : Import testing utilities (devTest, prodTest, devAndProductionTest, Dev, Client) from test/bake/bake-harness.ts
Applied to files:
test/regression/issue/decorator-metadata-unions.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/esm.test.ts : esm.test.ts should cover ESM feature behavior in development mode
Applied to files:
test/regression/issue/decorator-metadata-unions.test.tssrc/bundler/ParseTask.zigsrc/resolver/resolver.zigtest/regression/issue/decorator-metadata-strict-null-checks.test.ts
📚 Learning: 2025-10-12T02:22:34.373Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.373Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use utilities like `describe.each`, `toMatchSnapshot`, and lifecycle hooks (`beforeAll`, `beforeEach`, `afterEach`) and track resources for cleanup
Applied to files:
test/regression/issue/decorator-metadata-unions.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/*.test.ts : Write Dev Server and HMR tests in test/bake/dev/*.test.ts using devTest from the shared harness
Applied to files:
test/regression/issue/decorator-metadata-unions.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/css.test.ts : css.test.ts should contain CSS bundling tests in dev mode
Applied to files:
test/regression/issue/decorator-metadata-unions.test.tstest/regression/issue/decorator-metadata-strict-null-checks.test.ts
📚 Learning: 2025-10-12T02:22:34.373Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.373Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Organize tests with `describe` blocks to group related tests
Applied to files:
test/regression/issue/decorator-metadata-unions.test.ts
📚 Learning: 2025-11-06T00:58:23.955Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24417
File: test/js/bun/spawn/spawn.test.ts:903-918
Timestamp: 2025-11-06T00:58:23.955Z
Learning: In Bun test files, `await using` with spawn() is appropriate for long-running processes that need guaranteed cleanup on scope exit or when explicitly testing disposal behavior. For short-lived processes that exit naturally (e.g., console.log scripts), the pattern `const proc = spawn(...); await proc.exited;` is standard and more common, as evidenced by 24 instances vs 4 `await using` instances in test/js/bun/spawn/spawn.test.ts.
Applied to files:
test/regression/issue/decorator-metadata-unions.test.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/cli/**/*.{js,ts} : When testing Bun as a CLI, use spawn with bunExe() and bunEnv from harness, and capture stdout/stderr via pipes
Applied to files:
test/regression/issue/decorator-metadata-unions.test.ts
📚 Learning: 2025-09-02T05:33:37.517Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 22323
File: test/js/web/websocket/websocket-subprotocol.test.ts:74-75
Timestamp: 2025-09-02T05:33:37.517Z
Learning: In Bun's runtime, `await using` with Node.js APIs like `net.createServer()` is properly supported and should not be replaced with explicit cleanup. Bun has extended Node.js APIs with proper async dispose support.
Applied to files:
test/regression/issue/decorator-metadata-unions.test.ts
📚 Learning: 2025-10-12T02:22:34.373Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.373Z
Learning: Applies to test/{test/**/*.test.{ts,js,jsx,tsx,mjs,cjs},test/js/node/test/{parallel,sequential}/*.js} : When spawning Bun in tests, use `bunExe()` and `bunEnv` from `harness`
Applied to files:
test/regression/issue/decorator-metadata-unions.test.tstest/regression/issue/decorator-metadata-strict-null-checks.test.ts
📚 Learning: 2025-10-26T01:32:04.844Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 24082
File: test/cli/test/coverage.test.ts:60-112
Timestamp: 2025-10-26T01:32:04.844Z
Learning: In the Bun repository test files (test/cli/test/*.test.ts), when spawning Bun CLI commands with Bun.spawnSync for testing, prefer using stdio: ["inherit", "inherit", "inherit"] to inherit stdio streams rather than piping them.
Applied to files:
test/regression/issue/decorator-metadata-unions.test.ts
📚 Learning: 2025-10-12T02:22:34.373Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.373Z
Learning: Applies to test/{test/**/*.test.{ts,js,jsx,tsx,mjs,cjs},test/js/node/test/{parallel,sequential}/*.js} : Prefer async/await; for single callbacks, use `Promise.withResolvers()`
Applied to files:
test/regression/issue/decorator-metadata-unions.test.ts
📚 Learning: 2025-10-12T02:22:34.373Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.373Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Prefer concurrent tests (`test.concurrent`/`describe.concurrent`) over sequential when feasible
Applied to files:
test/regression/issue/decorator-metadata-unions.test.ts
📚 Learning: 2025-10-12T02:22:34.373Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.373Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `bun:test` for files ending with `*.test.{ts,js,jsx,tsx,mjs,cjs}`
Applied to files:
test/regression/issue/decorator-metadata-unions.test.tstest/regression/issue/decorator-metadata-strict-null-checks.test.ts
📚 Learning: 2025-10-26T05:04:50.692Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-26T05:04:50.692Z
Learning: Applies to test/**/*.{ts,tsx} : Avoid shell commands in tests (e.g., find, grep); use Bun's Glob and built-in tools
Applied to files:
test/regression/issue/decorator-metadata-unions.test.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/**/*.{js,ts} : Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test
Applied to files:
test/regression/issue/decorator-metadata-unions.test.tstest/regression/issue/decorator-metadata-strict-null-checks.test.ts
📚 Learning: 2025-10-08T13:48:02.430Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 23373
File: test/js/bun/tarball/extract.test.ts:107-111
Timestamp: 2025-10-08T13:48:02.430Z
Learning: In Bun's test runner, use `expect(async () => { await ... }).toThrow()` to assert async rejections. Unlike Jest/Vitest, Bun does not require `await expect(...).rejects.toThrow()` - the async function wrapper with `.toThrow()` is the correct pattern for async error assertions in Bun tests.
Applied to files:
test/regression/issue/decorator-metadata-unions.test.ts
📚 Learning: 2025-09-17T23:42:05.812Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 22568
File: docs/api/redis.md:240-246
Timestamp: 2025-09-17T23:42:05.812Z
Learning: The RedisClient.duplicate() method in Bun's Redis implementation returns Promise<RedisClient>, not RedisClient synchronously, so await is required when calling it.
Applied to files:
test/regression/issue/decorator-metadata-unions.test.ts
📚 Learning: 2025-10-12T02:22:34.373Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.373Z
Learning: Applies to test/{test/**/*.test.{ts,js,jsx,tsx,mjs,cjs},test/js/node/test/{parallel,sequential}/*.js} : Do not set explicit test timeouts; Bun already has timeouts
Applied to files:
test/regression/issue/decorator-metadata-unions.test.tstest/regression/issue/decorator-metadata-strict-null-checks.test.ts
📚 Learning: 2025-10-19T02:44:46.354Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/context-propagation.test.ts:1-1
Timestamp: 2025-10-19T02:44:46.354Z
Learning: In the Bun repository, standalone packages under packages/ (e.g., bun-vscode, bun-inspector-protocol, bun-plugin-yaml, bun-plugin-svelte, bun-debug-adapter-protocol, bun-otel) co-locate their tests with package source code using *.test.ts files. This follows standard npm/monorepo patterns. The test/ directory hierarchy (test/js/bun/, test/cli/, test/js/node/) is reserved for testing Bun's core runtime APIs and built-in functionality, not standalone packages.
Applied to files:
test/regression/issue/decorator-metadata-unions.test.ts
📚 Learning: 2025-10-18T05:23:24.403Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: test/js/bun/telemetry-server.test.ts:91-100
Timestamp: 2025-10-18T05:23:24.403Z
Learning: In the Bun codebase, telemetry tests (test/js/bun/telemetry-*.test.ts) should focus on telemetry API behavior: configure/disable/isEnabled, callback signatures and invocation, request ID correlation, and error handling. HTTP protocol behaviors like status code normalization (e.g., 200 with empty body → 204) should be tested in HTTP server tests (test/js/bun/http/), not in telemetry tests. Keep separation of concerns: telemetry tests verify the telemetry API contract; HTTP tests verify HTTP semantics.
Applied to files:
test/regression/issue/decorator-metadata-unions.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/bake-harness.ts : Provide and maintain shared test utilities: devTest, prodTest, devAndProductionTest, Dev, Client, and helpers in the harness
Applied to files:
test/regression/issue/decorator-metadata-unions.test.tstest/regression/issue/decorator-metadata-strict-null-checks.test.ts
📚 Learning: 2025-10-25T17:20:19.041Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 24063
File: test/js/bun/telemetry/server-header-injection.test.ts:5-20
Timestamp: 2025-10-25T17:20:19.041Z
Learning: In the Bun telemetry codebase, tests are organized into two distinct layers: (1) Internal API tests in test/js/bun/telemetry/ use numeric InstrumentKind enum values to test Zig↔JS injection points and low-level integration; (2) Public API tests in packages/bun-otel/test/ use string InstrumentKind values ("http", "fetch", etc.) to test the public-facing BunSDK and instrumentation APIs. This separation allows internal tests to use efficient numeric enums for refactoring flexibility while the public API maintains a developer-friendly string-based interface.
Applied to files:
test/regression/issue/decorator-metadata-unions.test.ts
📚 Learning: 2025-09-02T19:17:26.376Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 0
File: :0-0
Timestamp: 2025-09-02T19:17:26.376Z
Learning: In Bun's Zig codebase, when handling error unions where the same cleanup operation (like `rawFree`) needs to be performed regardless of success or failure, prefer using boolean folding with `else |err| switch (err)` over duplicating the cleanup call in multiple switch branches. This approach avoids code duplication while maintaining compile-time error checking.
Applied to files:
src/ast/TypeScript.zig
📚 Learning: 2025-08-30T00:13:36.815Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/**/*.zig : Constructors and prototype methods should return bun.JSError!JSC.JSValue to integrate Zig error handling with JS exceptions
Applied to files:
src/bun.js/ModuleLoader.zig
📚 Learning: 2025-10-26T05:04:50.692Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-26T05:04:50.692Z
Learning: Applies to src/bun.js/bindings/**/*.cpp : Add iso subspaces for classes with C++ fields in JavaScriptCore bindings
Applied to files:
src/bun.js/ModuleLoader.zig
📚 Learning: 2025-10-04T09:52:49.414Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-10-04T09:52:49.414Z
Learning: Applies to src/js/bun/**/*.{ts,js} : Place Bun-specific modules (e.g., bun:ffi, bun:sqlite) under bun/
Applied to files:
src/bun.js/ModuleLoader.zig
📚 Learning: 2025-10-18T01:49:31.037Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23755
File: src/bun.js/api/bun/socket/SocketConfig.bindv2.ts:58-58
Timestamp: 2025-10-18T01:49:31.037Z
Learning: In Bun's bindgenv2 TypeScript bindings (e.g., src/bun.js/api/bun/socket/SocketConfig.bindv2.ts), the pattern `b.String.loose.nullable.loose` is intentional and not a duplicate. The first `.loose` applies to the String type (loose string conversion), while the second `.loose` applies to the nullable (loose nullable, treating all falsy values as null rather than just null/undefined).
Applied to files:
src/bun.js/ModuleLoader.zig
📚 Learning: 2025-10-15T04:01:16.478Z
Learnt from: AmanVarshney01
Repo: oven-sh/bun PR: 23580
File: docs/guides/ecosystem/prisma.md:21-26
Timestamp: 2025-10-15T04:01:16.478Z
Learning: For Prisma with Bun runtime and Rust-free client (engineType "client"), when using local SQLite files (file:// URLs), use either prisma/adapter-better-sqlite3 or the Bun-native synapsenwerkstatt/prisma-bun-sqlite-adapter. The prisma/adapter-libsql adapter is only for remote libSQL/Turso endpoints and will not work with local SQLite file URLs.
Applied to files:
src/bun.js/ModuleLoader.zig
📚 Learning: 2025-10-04T09:52:49.414Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-10-04T09:52:49.414Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Do not use ESM import syntax; write modules as CommonJS with export default { ... }
Applied to files:
src/resolver/resolver.zig
📚 Learning: 2025-10-19T02:52:37.412Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/tsconfig.json:1-15
Timestamp: 2025-10-19T02:52:37.412Z
Learning: In the Bun repository, packages under packages/ (e.g., bun-otel) can follow a TypeScript-first pattern where package.json exports point directly to .ts files (not compiled .js files). Bun natively runs TypeScript, so consumers import .ts sources directly and receive full type information without needing compiled .d.ts declaration files. For such packages, adding "declaration": true or "outDir" in tsconfig.json is unnecessary and would break the export structure.
<!-- [remove_learning]
ceedde95-980e-4898-a2c6-40ff73913664
Applied to files:
src/resolver/resolver.zig
📚 Learning: 2025-10-04T09:52:49.414Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-10-04T09:52:49.414Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Prefer JSC intrinsics/private $ APIs for performance (e.g., $Array.from, map.$set, $newArrayWithSize, $debug, $assert)
Applied to files:
src/resolver/resolver.zig
📚 Learning: 2025-10-12T02:22:34.373Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.373Z
Learning: Applies to test/regression/issue/*.test.ts : Place regression tests for specific issues in `/test/regression/issue/${issueNumber}.test.ts`
Applied to files:
test/regression/issue/decorator-metadata-strict-null-checks.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/sourcemap.test.ts : sourcemap.test.ts should verify source map correctness in dev
Applied to files:
test/regression/issue/decorator-metadata-strict-null-checks.test.ts
📚 Learning: 2025-10-26T05:04:50.692Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-26T05:04:50.692Z
Learning: Applies to test/regression/issue/*.test.ts : Place tests for specific numbered GitHub issues in test/regression/issue/${issueNumber}.test.ts and ensure the issue number is real
Applied to files:
test/regression/issue/decorator-metadata-strict-null-checks.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/react-spa.test.ts : react-spa.test.ts should contain React SPA, react-refresh, and basic server component transform tests
Applied to files:
test/regression/issue/decorator-metadata-strict-null-checks.test.ts
📚 Learning: 2025-10-26T05:04:50.692Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-26T05:04:50.692Z
Learning: Applies to test/**/*.test.{ts,tsx} : Never write tests that assert absence of crashes (e.g., no "panic" or "uncaught exception") in output
Applied to files:
test/regression/issue/decorator-metadata-strict-null-checks.test.ts
📚 Learning: 2025-10-12T02:22:34.373Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.373Z
Learning: Applies to test/{test/**/*.test.{ts,js,jsx,tsx,mjs,cjs},test/js/node/test/{parallel,sequential}/*.js} : Always check exit codes and error scenarios in tests (e.g., spawned processes should assert non-zero on failure)
Applied to files:
test/regression/issue/decorator-metadata-strict-null-checks.test.ts
📚 Learning: 2025-10-26T05:04:50.692Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-26T05:04:50.692Z
Learning: Applies to test/**/*.test.{ts,tsx} : Do not use setTimeout in tests; await conditions instead to avoid flakiness
Applied to files:
test/regression/issue/decorator-metadata-strict-null-checks.test.ts
🧬 Code graph analysis (2)
test/regression/issue/decorator-metadata-unions.test.ts (1)
test/harness.ts (3)
tempDir(277-284)bunExe(102-105)nodeExe(107-109)
test/regression/issue/decorator-metadata-strict-null-checks.test.ts (1)
test/harness.ts (2)
tempDir(277-284)bunExe(102-105)
🔇 Additional comments (14)
src/ast/skipTypescript.zig (1)
632-641: Confirm correct parser-options path for strict_null_checksThis uses p.options.features.strict_null_checks. If the parser’s options expose strict_null_checks at the top level (not under .features), this will fail to compile. Adjust to the correct path accordingly.
Apply if needed:
- result.mergeUnion(left, p.options.features.strict_null_checks); + result.mergeUnion(left, p.options.strict_null_checks);src/bun.js/ModuleLoader.zig (2)
232-236: Propagating strict_null_checks into ParseOptionsGood: threads the flag into the parser pipeline.
658-676: Whitespace-only change in generated SQLite module snippetsLooks like formatting only; no behavior change.
Also applies to: 678-686
src/bun.js/RuntimeTranspilerStore.zig (1)
384-390: Async path also propagates strict_null_checksSymmetric with sync path; LGTM.
src/options.zig (1)
1724-1724: strict_null_checks option is properly wired end-to-endVerification confirms the option flows correctly from tsconfig parsing → resolver → transpiler → bundler → parser features. The default of false preserves existing behavior, and the feature is propagated through ResolveResult and ParseTask into the type checking logic.
src/transpiler.zig (4)
466-469: Good: tsconfig.strictNullChecks → options.strict_null_checksThis aligns configuration at the top-level. LGTM.
1090-1093: Good: parser features.strict_null_checks wiredThis completes the end-to-end threading into js_parser. LGTM.
949-952: JSTranspiler.zig: Two ParseOptions literals silently use strict_null_checks default — verify intentionalTwo public API entry points in
src/bun.js/api/JSTranspiler.zigcreate ParseOptions without explicitly assigningstrict_null_checks:
- Line 502 (transpile method): missing
strict_null_checks(also missingemit_decorator_metadata)- Line 749 (getParseResult function): missing
strict_null_checks(also missingemit_decorator_metadata)All other 13+ ParseOptions instances across the codebase explicitly assign this field. These two cases inherit some transpiler config (e.g.,
jsx,macro_map) but skip both decorator and strict_null_checks fields, suggesting an inconsistency worth confirming—either intentional API design or an oversight from refactoring.
632-636: strict_null_checks is properly threaded through all resolve flows—verified and approved.The field is correctly initialized from tsconfig via
isStrictNullChecksEnabled()in the mainresolve()function (line 1002), propagated through parent configs (line 4223), and properly passed to the parser in transpiler.zig. Entry point flows delegate toresolve()recursively, ensuring consistent handling. The safe default value (false) handles cases where no tsconfig is found.src/ast/TypeScript.zig (1)
93-122: Add symmetric strictNullChecks handling when left operand is null/undefinedThe current implementation only checks
strict_null_checkswhenresultis null/undefined, but collapses to Object unconditionally whenresultis a concrete type andleftis null/undefined. This asymmetry can manifest with multi-part unions likeA | null | Bdepending on parse order.The proposed fix correctly adds the symmetric case to handle both argument orders. Existing tests cover strict_null_checks behavior with
Type | nullpatterns, but lack coverage for reverse orders (null | Type).- pub fn mergeUnion(result: *@This(), left: @This(), strict_null_checks: bool) void { + pub fn mergeUnion(result: *@This(), left: @This(), strict_null_checks: bool) void { if (left != .m_none) { if (std.meta.activeTag(result.*) != std.meta.activeTag(left)) { - result.* = switch (result.*) { - .m_never => left, - // When strictNullChecks is enabled, unions with null or undefined should emit Object - // This matches TypeScript's behavior with strictNullChecks: true - // When strictNullChecks is disabled, we emit the actual type (which may cause TDZ errors at runtime) - .m_null, .m_undefined => switch (left) { - .m_never, .m_undefined, .m_null => left, - else => if (strict_null_checks) .m_object else left, - }, - else => .m_object, - }; + result.* = switch (result.*) { + .m_never => left, + .m_null, .m_undefined => switch (left) { + .m_never, .m_undefined, .m_null => left, + else => if (strict_null_checks) .m_object else left, + }, + else => switch (left) { + .m_null, .m_undefined => if (strict_null_checks) .m_object else result.*, + else => .m_object, + }, + }; } else { switch (result.*) { .m_identifier => |ref| {Verify the fix by running the proposed tests for both
A | nullandnull | Aunion orders under bothstrictNullChecks: trueandfalsesettings to confirm symmetric behavior.test/regression/issue/decorator-metadata-unions.test.ts (2)
240-287: LGTM! Properly implemented test.This test correctly uses
await usingfor process management, avoids explicit timeouts, and properly validates the ORM circular reference pattern without TDZ errors.
43-50: Useawait usingfor automatic resource cleanup.Per past review comments and for consistency with the test process pattern, the install process should use
await usingfor proper automatic cleanup.Apply this diff:
- const installProc = Bun.spawn({ + await using installProc = Bun.spawn({ cmd: [bunExe(), "install"], cwd: String(dir), env: bunEnv, stdout: "pipe", stderr: "pipe", }); - await installProc.exited;⛔ Skipped due to learnings
Learnt from: markovejnovic Repo: oven-sh/bun PR: 24417 File: test/js/bun/spawn/spawn.test.ts:903-918 Timestamp: 2025-11-06T00:58:23.955Z Learning: In Bun test files, `await using` with spawn() is appropriate for long-running processes that need guaranteed cleanup on scope exit or when explicitly testing disposal behavior. For short-lived processes that exit naturally (e.g., console.log scripts), the pattern `const proc = spawn(...); await proc.exited;` is standard and more common, as evidenced by 24 instances vs 4 `await using` instances in test/js/bun/spawn/spawn.test.ts.Learnt from: CR Repo: oven-sh/bun PR: 0 File: test/CLAUDE.md:0-0 Timestamp: 2025-10-12T02:22:34.373Z Learning: Applies to test/{test/**/*.test.{ts,js,jsx,tsx,mjs,cjs},test/js/node/test/{parallel,sequential}/*.js} : Prefer async/await; for single callbacks, use `Promise.withResolvers()`Learnt from: theshadow27 Repo: oven-sh/bun PR: 24063 File: packages/bun-otel/test/context-propagation.test.ts:1-7 Timestamp: 2025-10-30T03:48:10.513Z Learning: In Bun test files, `using` declarations at the describe block level execute during module load/parsing, not during test execution. This means they acquire and dispose resources before any tests run. For test-scoped resource management, use beforeAll/afterAll hooks instead. The pattern `beforeAll(beforeUsingEchoServer); afterAll(afterUsingEchoServer);` is correct for managing ref-counted test resources like the EchoServer in packages/bun-otel/test/ - the using block pattern should not be used at describe-block level for test resources. <!-- [/add_learning]Learnt from: CR Repo: oven-sh/bun PR: 0 File: test/CLAUDE.md:0-0 Timestamp: 2025-10-12T02:22:34.373Z Learning: Applies to test/{test/**/*.test.{ts,js,jsx,tsx,mjs,cjs},test/js/node/test/{parallel,sequential}/*.js} : When spawning Bun in tests, use `bunExe()` and `bunEnv` from `harness`Learnt from: CR Repo: oven-sh/bun PR: 0 File: test/CLAUDE.md:0-0 Timestamp: 2025-10-12T02:22:34.373Z Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `bun:test` for files ending with `*.test.{ts,js,jsx,tsx,mjs,cjs}`Learnt from: CR Repo: oven-sh/bun PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-10-26T05:04:50.692Z Learning: Applies to test/**/*.test.{ts,tsx} : Do not use setTimeout in tests; await conditions instead to avoid flakinessLearnt from: CR Repo: oven-sh/bun PR: 0 File: test/CLAUDE.md:0-0 Timestamp: 2025-10-12T02:22:34.373Z Learning: Applies to test/{test/**/*.test.{ts,js,jsx,tsx,mjs,cjs},test/js/node/test/{parallel,sequential}/*.js} : Do not set explicit test timeouts; Bun already has timeoutsLearnt from: CR Repo: oven-sh/bun PR: 0 File: .cursor/rules/writing-tests.mdc:0-0 Timestamp: 2025-08-30T00:12:56.803Z Learning: Applies to test/cli/**/*.{js,ts} : When testing Bun as a CLI, use spawn with bunExe() and bunEnv from harness, and capture stdout/stderr via pipesLearnt from: CR Repo: oven-sh/bun PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-10-26T05:04:50.692Z Learning: Applies to test/**/*.test.{ts,tsx} : Prefer snapshot assertions and use normalizeBunSnapshot for snapshot output in testsLearnt from: Jarred-Sumner Repo: oven-sh/bun PR: 24082 File: test/cli/test/coverage.test.ts:60-112 Timestamp: 2025-10-26T01:32:04.844Z Learning: In the Bun repository test files (test/cli/test/*.test.ts), when spawning Bun CLI commands with Bun.spawnSync for testing, prefer using stdio: ["inherit", "inherit", "inherit"] to inherit stdio streams rather than piping them.src/resolver/resolver.zig (1)
168-169: Strict null checks propagation looks correct.Result flag, extends merge, and finalizeResult all align with TS precedence (explicit strictNullChecks, else strict). LGTM.
Please confirm other option carriers (e.g., Transpiler options initialization) source this value from Resolver.Result for per-file correctness.
Also applies to: 1000-1004, 4222-4225
src/resolver/tsconfig_json.zig (1)
45-48: TS config parsing and precedence for strict null checks look good.Explicit strictNullChecks overrides strict; default false otherwise. Implementation is clear and aligns with intended behavior.
Also applies to: 53-69, 201-214
|
Closing as stale: this PR predates the Rust rewrite. Every If the underlying change is still wanted, it will need to be redone against the current Rust/C++ tree. Apologies for the churn, and thank you for the contribution. |
3 participants
What does this PR do?
See "Bun matches TypeScript with strictNullChecks" test, tldr is tsc emits
ObjectforClass | nullunions whereas Bun emitsClass. This is only an issue when strict null checking is enabled, otherwise tsc also emitsClassbecause the union is disregarded.This issue leads to ReferenceError when using ORMs, see this issue comment (not the exact scenario but same result):
#4136 (comment)
Fwiw I do think Bun is more "correct" in that most cases you would expect the type metadata to reflect
ClassnotObjecteven with a null union, but in the spirit of compatibility I guess this is the right path. The only alternative I can see is wrapping the generatedtypeofin a try/catch and returningObjectif the class is not defined andClassif it is defined, but that is more complicated and weird than this solution that perfectly reflects tsc behavior.How did you verify your code works?
There are multiple tests, I also tested it on the reference project I was having the issue with and it works as well.