Fix TypeScript syntax not working with 'ts' loader in BunPlugin by robobun · Pull Request #22460 · oven-sh/bun

robobun · 2025-09-07T07:38:32Z

Summary

Fixes #12548 - TypeScript syntax doesn't work in BunPlugin when using loader: 'ts'

The Problem

When creating a virtual module with build.module() and specifying loader: 'ts', TypeScript syntax like import { type TSchema } would fail to parse with errors like:

error: Expected "}" but found "TSchema"
error: Expected "from" but found "}"

The same code worked fine when using loader: 'tsx', indicating the TypeScript parser wasn't being configured correctly for .ts files.

Root Cause

The bug was caused by an enum value mismatch between C++ and Zig:

Before (Incorrect)

C++ (headers-handwritten.h): jsx=0, js=1, ts=2, tsx=3, ...
Zig API (api/schema.zig): jsx=1, js=2, ts=3, tsx=4, ...
Zig Internal (options.zig): jsx=0, js=1, ts=2, tsx=3, ...

When a plugin returned loader: 'ts', the C++ code correctly parsed the string "ts" and set BunLoaderTypeTS=2. However, when this value was passed to Zig's Bun__transpileVirtualModule function (which expects api.Loader), the value 2 was interpreted as api.Loader.js instead of api.Loader.ts, causing the TypeScript parser to not be enabled.

Design Context

The codebase has two loader enum systems by design:

api.Loader: External API interface used for C++/Zig communication
options.Loader: Internal representation used within Zig

The conversion between them happens via options.Loader.fromAPI() and .toAPI() functions. The C++ layer should use api.Loader values since that's what the interface functions expect.

The Fix

Aligned enum values: Updated the BunLoaderType constants in headers-handwritten.h to match the values in api/schema.zig, ensuring C++ and Zig agree on the enum values
Removed unnecessary assertion: Removed the assertion that plugin_runner must be non-null for virtual modules, as it's not actually required for modules created via build.module()
Added regression test: Created comprehensive test in test/regression/issue/12548.test.ts that verifies TypeScript syntax works correctly with the 'ts' loader

Testing

New Tests Pass

✅ test/regression/issue/12548.test.ts - 2 tests verifying TypeScript type imports work with 'ts' loader

Existing Tests Still Pass

✅ test/js/bun/plugin/plugins.test.ts - 28 pass
✅ test/bundler/bundler_plugin.test.ts - 52 pass
✅ test/bundler/bundler_loader.test.ts - 27 pass
✅ test/bundler/esbuild/loader.test.ts - 10 pass
✅ test/bundler/bundler_plugin_chain.test.ts - 13 pass

Manual Verification

// This now works correctly with loader: 'ts'
Bun.plugin({
  setup(build) {
    build.module('hi', () => ({
      contents: "import { type TSchema } from '@sinclair/typebox'",
      loader: 'ts',  // ✅ Works now (previously failed)
    }))
  },
})

🤖 Generated with Claude Code

The issue was caused by a mismatch between the BunLoaderType enum values in C++ (headers-handwritten.h) and the api.Loader enum values in Zig (api/schema.zig). When a plugin returned `loader: 'ts'`, the C++ code would set BunLoaderTypeTS=2, but when this value was passed to Zig as api.Loader, it was interpreted as api.Loader.js (which is 2 in the API schema), causing TypeScript syntax to not be recognized. Fixed by aligning the C++ BunLoaderType values with the api/schema.zig Loader enum values. Also removed an unnecessary assertion that plugin_runner must be non-null for virtual modules, as plugin_runner is not required for modules created via build.module(). 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

robobun · 2025-09-07T07:38:40Z

^{Updated 9:38 PM PT - Sep 7th, 2025}

❌ @Jarred-Sumner, your commit d5ea237 has 2 failures in Build #25338:

test/bake/dev/css.test.ts - 2 failing on 🍎 13 aarch64
test/cli/run/filter-workspace.test.ts - 1 failing on 🍎 14 aarch64

🧪 To try this PR locally:

bunx bun-pr 22460

That installs a local version of the PR into your bun-22460 executable, so you can run:

bun-22460 --bun

coderabbitai · 2025-09-07T07:43:52Z

Walkthrough

Removed a null assertion for plugin_runner in virtual-module transpilation, remapped BunLoaderType numeric values to match Zig, added regression tests validating TypeScript loader ('ts') for Bun.plugin virtual modules, and split virtual-module removals in BunPlugin.cpp into staged operations with per-step exception checks.

Changes

Cohort / File(s)	Summary
Virtual module transpile guard `src/bun.js/ModuleLoader.zig`	Removed runtime assertion that `jsc_vm.plugin_runner` must be non-null in `Bun__transpileVirtualModule` and added a comment stating the plugin runner is not required for `build.module()` virtual modules.
Loader enum alignment `src/bun.js/bindings/headers-handwritten.h`	Remapped `BunLoaderType` numeric constants to align with Zig's `Loader` enum (JSX=1, JS=2, TS=3, TSX=4, CSS=5, FILE=6, JSON=7, JSONC=8, TOML=9, WASM=10, NAPI=11, YAML=19); `None` unchanged.
Regression tests for TS in plugins `test/regression/issue/12548.test.ts`	Added tests confirming virtual modules provided by `Bun.plugin` with loader `'ts'` accept TypeScript syntax and type-only imports/exports and produce expected runtime outputs.
Staged virtual-module removals `src/bun.js/bindings/BunPlugin.cpp`	Split the removal of virtual module entries into two staged removals (requireMap then esmRegistryMap) with intermediate exception checks and additional local map pointers.

Assessment against linked issues

Objective	Addressed	Explanation
TypeScript syntax works in BunPlugin virtual modules with loader 'ts' (#12548)	✅

Assessment — Out-of-scope changes

Code Change	Explanation
Staged removal and added exception checks in virtual-module removal (src/bun.js/bindings/BunPlugin.cpp)	This change adjusts internal error handling and removal order for require/ESM registries and does not relate to enabling TypeScript syntax support for Bun.plugin virtual modules described in #12548.

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.

✨ Finishing Touches

📝 Generate Docstrings

🧪 Generate unit tests

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch claude/fix-ts-loader-plugin-issue-12548

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (3)

test/regression/issue/12548.test.ts (3)
2-2: Remove unused import or use it for snapshots

normalizeBunSnapshot is imported but unused. Either remove it or switch assertions to snapshots for stability across environments.
-import { bunEnv, bunExe, normalizeBunSnapshot, tempDir } from "harness";
+import { bunEnv, bunExe, tempDir } from "harness";
11-13: Make the assertion deterministic (avoid console formatting differences)

console.log(require('virtual-ts-module')) depends on runtime formatting of module namespace objects. Log the exported value directly (or JSON) for stable output.
-      // This should work with 'ts' loader
-      console.log(require('virtual-ts-module'));
+      // This should work with 'ts' loader
+      console.log(require('virtual-ts-module').test);
And update the expectation below (Line 42-43):
-  expect(stdout).toContain('test: "works"');
+  expect(stdout).toContain("works");
45-61: Solid coverage; consider adding a TSX case to guard against regressions

You cover 'ts'; add a similar test for loader: 'tsx' with a type-only import and a trivial JSX or TSX parse to ensure both enum paths are correct.

I can draft a third test mirroring this one with loader: 'tsx' and a no-op fragment to confirm parsing.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 2daf7ed and b7d6751.

📒 Files selected for processing (3)

src/bun.js/ModuleLoader.zig (1 hunks)
src/bun.js/bindings/headers-handwritten.h (1 hunks)
test/regression/issue/12548.test.ts (1 hunks)

🧰 Additional context used

📓 Path-based instructions (11)

test/**

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

Place all tests under the test/ directory

Files:

test/regression/issue/12548.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/12548.test.ts

test/**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

test/**/*.test.{ts,tsx}: Tests must be named with the suffix .test.ts or .test.tsx and live under the test/ directory
In tests, always use port: 0 and never hardcode ports or use custom random-port utilities
Prefer snapshot tests using normalizeBunSnapshot(...) over direct string equality on stdout/stderr
Do not write tests that assert absence of 'panic', 'uncaught exception', or similar strings in output
Avoid shelling out to tools like find or grep in tests; use Bun.Glob and built-in utilities

Files:

test/regression/issue/12548.test.ts

test/regression/issue/**

📄 CodeRabbit inference engine (CLAUDE.md)

Place regression tests (one per bug fix) under test/regression/issue/

Place regression tests under test/regression/issue/ and organize by issue number

Files:

test/regression/issue/12548.test.ts

**/*.{ts,tsx,js}

📄 CodeRabbit inference engine (CLAUDE.md)

Run Prettier to format JS/TS files (bun run prettier)

Files:

test/regression/issue/12548.test.ts

test/**/*.test.ts

📄 CodeRabbit inference engine (test/CLAUDE.md)

test/**/*.test.ts: Name test files *.test.ts and use bun:test
Do not write flaky tests: never wait for arbitrary time; wait for conditions instead
Never hardcode port numbers in tests; use port: 0 to get a random port
When spawning Bun in tests, use bunExe() and bunEnv from harness
Prefer async/await in tests; for a single callback, use Promise.withResolvers()
Do not set explicit test timeouts; rely on Bun’s built-in timeouts
Use tempDir/tempDirWithFiles from harness for temporary files and directories in tests
For large/repetitive strings in tests, prefer Buffer.alloc(count, fill).toString() over "A".repeat(count)
Import common test utilities from harness (e.g., bunExe, bunEnv, tempDirWithFiles, tmpdirSync, platform checks, GC helpers)
In error tests, assert non-zero exit codes for failing processes and use toThrow for synchronous errors
Use describe blocks for grouping, describe.each for parameterized tests, snapshots with toMatchSnapshot, and lifecycle hooks (beforeAll, beforeEach, afterEach); track resources for cleanup in afterEach
Use using/await using with Bun resources (e.g., Bun.listen/connect/spawn/serve) to ensure cleanup in tests

Files:

test/regression/issue/12548.test.ts

src/**/*.zig

📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)

Implement debug logs in Zig using const log = bun.Output.scoped(.${SCOPE}, false); and invoking log("...", .{})

In Zig code, manage memory carefully: use appropriate allocators and defer for cleanup

Files:

src/bun.js/ModuleLoader.zig

**/*.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

Run zig-format to format Zig files (bun run zig-format)

Files:

src/bun.js/ModuleLoader.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/ModuleLoader.zig

**/*.{cpp,h}

📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)

**/*.{cpp,h}: When exposing a JS class with public Constructor and Prototype, define three C++ types: class Foo : public JSC::DestructibleObject (if it has C++ fields), class FooPrototype : public JSC::JSNonFinalObject, and class FooConstructor : public JSC::InternalFunction
If the class has C++ data members, inherit from JSC::DestructibleObject and provide proper destruction; if it has no C++ fields (only JS properties), avoid a class and use JSC::constructEmptyObject(vm, structure) with putDirectOffset
Prefer placing the subspaceFor implementation in the .cpp file rather than the header when possible

Files:

src/bun.js/bindings/headers-handwritten.h

**/*.{cpp,hpp,h,cc,cxx}

📄 CodeRabbit inference engine (CLAUDE.md)

Run clang-format to format C/C++ files (bun run clang-format)

Files:

src/bun.js/bindings/headers-handwritten.h

🧠 Learnings (21)

📓 Common learnings

Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/js/bun/**/*.{js,ts} : Place Bun API tests under test/js/bun/, separated by category (e.g., test/js/bun/glob/)

Learnt from: CR
PR: oven-sh/bun#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

Learnt from: CR
PR: oven-sh/bun#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

Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-09-03T17:10:13.486Z
Learning: Applies to test/**/*.test.ts : When spawning Bun in tests, use `bunExe()` and `bunEnv` from `harness`

Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-09-03T17:10:13.486Z
Learning: Applies to test/**/*.test.ts : Name test files `*.test.ts` and use `bun:test`

Learnt from: CR
PR: oven-sh/bun#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

Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-03T17:09:28.113Z
Learning: Applies to test/js/bun/** : Place Bun-specific API tests (http, crypto, ffi, shell, etc.) under test/js/bun/

Learnt from: CR
PR: oven-sh/bun#0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-09-07T05:41:52.546Z
Learning: Applies to src/js/bun/**/*.{js,ts} : Place Bun-specific modules (e.g., `bun:ffi`, `bun:sqlite`) under `bun/`

📚 Learning: 2025-08-30T00:09:39.100Z

Learnt from: CR
PR: oven-sh/bun#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/12548.test.ts

📚 Learning: 2025-09-03T17:10:13.486Z

Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-09-03T17:10:13.486Z
Learning: Applies to test/**/*.test.ts : Name test files `*.test.ts` and use `bun:test`

Applied to files:

test/regression/issue/12548.test.ts

📚 Learning: 2025-08-30T00:12:56.803Z

Learnt from: CR
PR: oven-sh/bun#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/12548.test.ts

📚 Learning: 2025-09-03T17:10:13.486Z

Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-09-03T17:10:13.486Z
Learning: Applies to test/**/*.test.ts : When spawning Bun in tests, use `bunExe()` and `bunEnv` from `harness`

Applied to files:

test/regression/issue/12548.test.ts

📚 Learning: 2025-08-30T00:12:56.803Z

Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/js/bun/**/*.{js,ts} : Place Bun API tests under test/js/bun/, separated by category (e.g., test/js/bun/glob/)

Applied to files:

test/regression/issue/12548.test.ts

📚 Learning: 2025-08-30T00:09:39.100Z

Learnt from: CR
PR: oven-sh/bun#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/12548.test.ts

📚 Learning: 2025-09-03T17:09:28.113Z

Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-03T17:09:28.113Z
Learning: Applies to test/js/bun/** : Place Bun-specific API tests (http, crypto, ffi, shell, etc.) under test/js/bun/

Applied to files:

test/regression/issue/12548.test.ts

📚 Learning: 2025-08-30T00:12:56.803Z

Learnt from: CR
PR: oven-sh/bun#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/12548.test.ts

📚 Learning: 2025-08-30T00:09:39.100Z

Learnt from: CR
PR: oven-sh/bun#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/12548.test.ts

📚 Learning: 2025-08-30T00:09:39.100Z

Learnt from: CR
PR: oven-sh/bun#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/12548.test.ts

📚 Learning: 2025-09-03T17:10:13.486Z

Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-09-03T17:10:13.486Z
Learning: Applies to test/**/*.test.ts : Import common test utilities from `harness` (e.g., `bunExe`, `bunEnv`, `tempDirWithFiles`, `tmpdirSync`, platform checks, GC helpers)

Applied to files:

test/regression/issue/12548.test.ts

📚 Learning: 2025-09-03T17:09:28.113Z

Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-03T17:09:28.113Z
Learning: Applies to test/**/*.test.{ts,tsx} : Prefer snapshot tests using normalizeBunSnapshot(...) over direct string equality on stdout/stderr

Applied to files:

test/regression/issue/12548.test.ts

📚 Learning: 2025-08-30T00:09:39.100Z

Learnt from: CR
PR: oven-sh/bun#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/12548.test.ts

📚 Learning: 2025-08-30T00:13:36.815Z

Learnt from: CR
PR: oven-sh/bun#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/ModuleLoader.zig
src/bun.js/bindings/headers-handwritten.h

📚 Learning: 2025-08-30T00:13:36.815Z

Learnt from: CR
PR: oven-sh/bun#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
src/bun.js/bindings/headers-handwritten.h

📚 Learning: 2025-08-30T00:13:36.815Z

Learnt from: CR
PR: oven-sh/bun#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/ModuleLoader.zig
src/bun.js/bindings/headers-handwritten.h

📚 Learning: 2025-08-30T00:13:36.815Z

Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/**/*.zig : Use parameter name globalObject (not ctx) and accept (*JSC.JSGlobalObject, *JSC.CallFrame) in binding methods/constructors

Applied to files:

src/bun.js/ModuleLoader.zig

📚 Learning: 2025-08-30T00:11:57.076Z

Learnt from: CR
PR: oven-sh/bun#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/ModuleLoader.zig
src/bun.js/bindings/headers-handwritten.h

📚 Learning: 2025-09-07T05:41:52.546Z

Learnt from: CR
PR: oven-sh/bun#0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-09-07T05:41:52.546Z
Learning: Applies to src/js/bun/**/*.{js,ts} : Place Bun-specific modules (e.g., `bun:ffi`, `bun:sqlite`) under `bun/`

Applied to files:

src/bun.js/ModuleLoader.zig

📚 Learning: 2025-09-07T05:41:52.546Z

Learnt from: CR
PR: oven-sh/bun#0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-09-07T05:41:52.546Z
Learning: Follow the build pipeline: Source TS/JS → Preprocessor → Bundler → C++ Headers; IDs assigned A–Z; `$` replaced with `__intrinsic__`; `require("x")` replaced with `$requireId(n)`; `export default` converted to `return`; `__intrinsic__` replaced with `@`; inlined into C++; modules loaded by numeric ID

Applied to files:

src/bun.js/bindings/headers-handwritten.h

🧬 Code graph analysis (1)

test/regression/issue/12548.test.ts (1)

test/harness.ts (1)

tempDir (277-284)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

GitHub Check: Format

🔇 Additional comments (1)

src/bun.js/ModuleLoader.zig (1)
1950-1952: Good removal: allow virtual modules without a plugin runner

Dropping the hard assertion avoids false crashes when build.module() feeds Bun__transpileVirtualModule directly. Behavior remains safe because this path doesn’t rely on plugin hooks; Bun__runVirtualModule still guards on plugin_runner when needed. No further changes required.

If you want extra safety, add a debug log when plugin_runner is null to aid diagnosing unexpected call paths:
   const jsc_vm = globalObject.bunVM();
-  // Plugin runner is not required for virtual modules created via build.module()
-  // bun.assert(jsc_vm.plugin_runner != null);
+  // Plugin runner is not required for virtual modules created via build.module()
+  if (comptime bun.Environment.allow_assert and jsc_vm.plugin_runner == null) {
+    debug("transpileVirtualModule called without plugin_runner", .{});
+  }

coderabbitai · 2025-09-07T07:43:56Z

src/bun.js/bindings/headers-handwritten.h

+// Must match api/schema.zig Loader enum values
+const BunLoaderType BunLoaderTypeJSX = 1;
+const BunLoaderType BunLoaderTypeJS = 2;
+const BunLoaderType BunLoaderTypeTS = 3;
+const BunLoaderType BunLoaderTypeTSX = 4;
+const BunLoaderType BunLoaderTypeCSS = 5;
+const BunLoaderType BunLoaderTypeFILE = 6;
+const BunLoaderType BunLoaderTypeJSON = 7;
+const BunLoaderType BunLoaderTypeJSONC = 8;
+const BunLoaderType BunLoaderTypeTOML = 9;
+const BunLoaderType BunLoaderTypeWASM = 10;
+const BunLoaderType BunLoaderTypeNAPI = 11;
+const BunLoaderType BunLoaderTypeYAML = 19;


💡 Verification agent

🧩 Analysis chain

Loader enum remap: verify ABI surface and add guardrails

Aligning BunLoaderType with api/schema.zig fixes the bug; however this is an ABI-visible change for any C/C++ callers passing numeric loader values. Confirm all call sites are recompiled and add compile-time/runtime checks where possible.

Add a small runtime self-test (once) that round-trips a few extensions through Bun__getDefaultLoader and compares with these constants.

Run clang-format as per guidelines.

Script to locate external uses that may need recompilation:

🏁 Script executed:

#!/bin/bash # Find BunLoaderType usages across the tree rg -n -C2 '\bBunLoaderType[A-Za-z0-9_]*\b|Bun__transpileVirtualModule\(|Bun__transpileFile\(' --type=cpp --type=c --type=h

Length of output: 12762

Ensure ABI stability for BunLoaderType constants

In src/bun.js/bindings/headers-handwritten.h, add _Static_assert checks immediately after the const BunLoaderType… definitions to verify each value matches the Zig enum in api/schema.zig.

In the exported C APIs (Bun__transpileVirtualModule, Bun__transpileFile, Bun__fetchBuiltinModule in headers-handwritten.h), validate that incoming loader is within the valid set (1–11, 19) and return an error or abort on out-of-range values.

Add a one-off runtime unit test in ModuleLoader.cpp that maps known file extensions through Bun__getDefaultLoader and asserts it returns the expected constant.

Run clang-format on all modified files.

The requireMap() and esmRegistryMap() getter functions can throw exceptions, but we weren't checking for them before calling remove() on the returned maps. This caused crashes when running with BUN_JSC_validateExceptionChecks=1. Fixed by: 1. Storing the map pointers in local variables 2. Adding RETURN_IF_EXCEPTION checks after each getter call 3. Then calling remove() on the validated pointers This ensures proper exception handling and prevents the crash seen in CI. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

coderabbitai

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/bun.js/bindings/BunPlugin.cpp (1)
939-946: Prevent double-free and stale resolver state in jsFunctionBunPluginClear

virtualModules is deleted but not nulled; subsequent calls risk double-free/UB.

onResolvePlugins.groups isn’t cleared (only namespaces), leaving dangling resolvers.

Apply:
   global->onLoadPlugins.fileNamespace.clear();
   global->onResolvePlugins.fileNamespace.clear();
   global->onLoadPlugins.groups.clear();
-  global->onResolvePlugins.namespaces.clear();
+  global->onResolvePlugins.groups.clear();
+  global->onResolvePlugins.namespaces.clear();

-  delete global->onLoadPlugins.virtualModules;
+  delete global->onLoadPlugins.virtualModules;
+  global->onLoadPlugins.virtualModules = nullptr;

🧹 Nitpick comments (3)

src/bun.js/bindings/BunPlugin.cpp (3)
1-2: Include root.h first to satisfy binding-file lint rules

Per repo guidelines, binding .cpp files should include root.h at the top.

Apply:
+ #include "root.h"
 #include "BunPlugin.h"
41-48: Use NeverDestroyed for the static regex to avoid manual new and potential teardown hazards

The lazy-initialized static pointer leaks by design. Prefer NeverDestroyed for clarity and safety.

Example:
-    static JSC::Yarr::RegularExpression* namespaceRegex = nullptr;
-    if (!namespaceRegex) {
-        namespaceRegex = new JSC::Yarr::RegularExpression("^([/@a-zA-Z0-9_\\-]+)$"_s);
-    }
-    return namespaceRegex->match(namespaceString) > -1;
+    static NeverDestroyed<JSC::Yarr::RegularExpression> namespaceRegex("^([/@a-zA-Z0-9_\\-]+)$"_s);
+    return namespaceRegex.get().match(namespaceString) > -1;
(You’ll need to include <wtf/NeverDestroyed.h>.)

181-185: Redundant RETURN_IF_EXCEPTION

The RETURN_IF_EXCEPTION(scope, {}) immediately before and after checking filterValue is duplicated and can be trimmed.
-    RETURN_IF_EXCEPTION(scope, {});
     if (filterValue) {
-        RETURN_IF_EXCEPTION(scope, {});
         if (filterValue.isCell() && filterValue.asCell()->inherits<JSC::RegExpObject>())
             filter = jsCast<JSC::RegExpObject*>(filterValue);
     }

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 9efa6cd and 9d6d0de.

📒 Files selected for processing (1)

src/bun.js/bindings/BunPlugin.cpp (1 hunks)

🧰 Additional context used

📓 Path-based instructions (4)

**/*.{cpp,h}

📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)

**/*.{cpp,h}: When exposing a JS class with public Constructor and Prototype, define three C++ types: class Foo : public JSC::DestructibleObject (if it has C++ fields), class FooPrototype : public JSC::JSNonFinalObject, and class FooConstructor : public JSC::InternalFunction
If the class has C++ data members, inherit from JSC::DestructibleObject and provide proper destruction; if it has no C++ fields (only JS properties), avoid a class and use JSC::constructEmptyObject(vm, structure) with putDirectOffset
Prefer placing the subspaceFor implementation in the .cpp file rather than the header when possible

Files:

src/bun.js/bindings/BunPlugin.cpp

**/*.cpp

📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)

**/*.cpp: Include "root.h" at the top of C++ binding files to satisfy lints
Define prototype properties using a const HashTableValue array and declare accessors/functions with JSC_DECLARE_* macros
Prototype classes should subclass JSC::JSNonFinalObject, provide create/createStructure, DECLARE_INFO, finishCreation that reifies static properties, and set mayBePrototype on the Structure
Custom getters should use JSC_DEFINE_CUSTOM_GETTER, jsDynamicCast to validate this, and throwThisTypeError on mismatch
Custom setters should use JSC_DEFINE_CUSTOM_SETTER, validate this via jsDynamicCast, and store via WriteBarrier/set semantics
Prototype functions should use JSC_DEFINE_HOST_FUNCTION, validate this with jsDynamicCast, and return encoded JSValue
Constructors should subclass JSC::InternalFunction, return internalFunctionSpace in subspaceFor, set the prototype property as non-configurable/non-writable, and provide create/createStructure
Provide a setup function that builds the Prototype, Constructor, and Structure, and assigns them to the LazyClassStructure initializer
Use the cached Structure via globalObject->m_.get(globalObject) when constructing instances
Expose constructors to Zig via an extern "C" function that returns the constructor from the LazyClassStructure
Provide an extern "C" Bun____toJS function that creates an instance using the cached Structure and returns an EncodedJSValue

Files:

src/bun.js/bindings/BunPlugin.cpp

src/bun.js/bindings/**/*.cpp

📄 CodeRabbit inference engine (CLAUDE.md)

src/bun.js/bindings/**/*.cpp: When implementing a JS class with a public constructor in C++ bindings, create Foo, FooPrototype, and FooConstructor classes
Define JS-visible properties using HashTableValue arrays
Add iso subspaces for classes with C++ fields
Cache class structures in ZigGlobalObject for bindings

Files:

src/bun.js/bindings/BunPlugin.cpp

**/*.{cpp,hpp,h,cc,cxx}

📄 CodeRabbit inference engine (CLAUDE.md)

Run clang-format to format C/C++ files (bun run clang-format)

Files:

src/bun.js/bindings/BunPlugin.cpp

🧠 Learnings (3)

📓 Common learnings

Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/js/bun/**/*.{js,ts} : Place Bun API tests under test/js/bun/, separated by category (e.g., test/js/bun/glob/)

📚 Learning: 2025-08-30T00:11:57.076Z

Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/registering-bun-modules.mdc:0-0
Timestamp: 2025-08-30T00:11:57.076Z
Learning: Applies to src/bun.js/bindings/BunObject+exports.h : When adding a new global module or getter (e.g., SMTP), ensure its name is added exactly once to FOR_EACH_GETTER

Applied to files:

src/bun.js/bindings/BunPlugin.cpp

📚 Learning: 2025-09-07T05:41:52.546Z

Learnt from: CR
PR: oven-sh/bun#0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-09-07T05:41:52.546Z
Learning: Applies to src/js/bun/**/*.{js,ts} : Place Bun-specific modules (e.g., `bun:ffi`, `bun:sqlite`) under `bun/`

Applied to files:

src/bun.js/bindings/BunPlugin.cpp

🔇 Additional comments (1)

src/bun.js/bindings/BunPlugin.cpp (1)

153-161: Good fix: clear CJS/ESM caches with per-step exception checks

Splitting the removals and guarding each with RETURN_IF_EXCEPTION eliminates stale entries and avoids cascading failures. Looks solid.

github-actions bot added the claude label Sep 7, 2025

[autofix.ci] apply automated fixes

9efa6cd

coderabbitai bot reviewed Sep 7, 2025

View reviewed changes

Claude Bot and others added 2 commits September 7, 2025 08:38

[autofix.ci] apply automated fixes

9d6d0de

coderabbitai bot reviewed Sep 7, 2025

View reviewed changes

Merge branch 'main' into claude/fix-ts-loader-plugin-issue-12548

d5ea237

Jarred-Sumner merged commit 63c4d8f into main Sep 8, 2025
56 of 60 checks passed

Jarred-Sumner deleted the claude/fix-ts-loader-plugin-issue-12548 branch September 8, 2025 04:43

coderabbitai bot mentioned this pull request Sep 8, 2025

Add build.module() support for Bun.build plugins #22486

Closed

This was referenced Sep 15, 2025

Fix crash when plugin onResolve returns undefined #22670

Merged

add OOM and stack checks in js_printer.zig #22472

Open

coderabbitai bot mentioned this pull request Sep 23, 2025

feat: implement bytes import type attribute #20839

Open

This was referenced Oct 4, 2025

fix: --port flag no longer affects file import URLs #23234

Closed

fix loaders used by Module._extensions #23291

Merged

feat: Add built-in CSV loader #19167

Open

This was referenced Oct 25, 2025

Refactor: Replace ResolvedSource with ModuleResult tagged union #24056

Closed

Refactor: Extract ModuleLoader components into separate files #24083

Merged

coderabbitai bot mentioned this pull request Nov 1, 2025

Add namespace and loader properties to runtime plugin onLoad callbacks #24282

Closed

coderabbitai bot mentioned this pull request Nov 28, 2025

fix(transpiler): preserve unicode characters in string literals #25179

Closed

3 tasks

This was referenced Dec 6, 2025

Failed to load external module with turbopack #25370

Open

TypeError: Expected CommonJS module to have a function wrapper. If you weren't messing around with Bun's internals, this is a bug in Bun #25398

Closed

This was referenced Dec 14, 2025

HTML Import Type Definitions Don't Reflect Import Attribute Behavior #25508

Closed

ESLint find-up resolution regression in Bun 1.3.4 #25522

Closed

Bun Incorrectly Merges TypeScript Path Mappings from Extended Configs #25622

Closed

coderabbitai bot mentioned this pull request Dec 22, 2025

TypeError: Expected CommonJS module to have a function wrapper when building Next.js 16 with Turbopack in Docker #25650

Closed

coderabbitai bot mentioned this pull request Jan 6, 2026

(feat): bun:test, module restoration and partial module mocking #25844

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix TypeScript syntax not working with 'ts' loader in BunPlugin#22460

Fix TypeScript syntax not working with 'ts' loader in BunPlugin#22460
Jarred-Sumner merged 5 commits intomainfrom
claude/fix-ts-loader-plugin-issue-12548

robobun commented Sep 7, 2025 •

edited

Loading

Uh oh!

robobun commented Sep 7, 2025 •

edited

Loading

Uh oh!

coderabbitai bot commented Sep 7, 2025 •

edited

Loading

Review ran into problems

Uh oh!

coderabbitai bot left a comment

Uh oh!

coderabbitai bot Sep 7, 2025

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

robobun commented Sep 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

The Problem

Root Cause

Before (Incorrect)

Design Context

The Fix

Testing

New Tests Pass

Existing Tests Still Pass

Manual Verification

Uh oh!

robobun commented Sep 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai bot commented Sep 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Assessment against linked issues

Assessment — Out-of-scope changes

Review ran into problems

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Sep 7, 2025

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

robobun commented Sep 7, 2025 •

edited

Loading

robobun commented Sep 7, 2025 •

edited

Loading

coderabbitai bot commented Sep 7, 2025 •

edited

Loading