From 63c4d8f68f2f10f00273cb5437a96d01292a2e1b Mon Sep 17 00:00:00 2001 From: robobun Date: Sun, 7 Sep 2025 21:43:38 -0700 Subject: [PATCH] Fix TypeScript syntax not working with 'ts' loader in BunPlugin (#22460) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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 1. **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 2. **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()` 3. **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 ```javascript // 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](https://claude.ai/code) --------- Co-authored-by: Claude Bot Co-authored-by: Claude Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Jarred Sumner --- src/bun.js/ModuleLoader.zig | 3 +- src/bun.js/bindings/BunPlugin.cpp | 9 ++- src/bun.js/bindings/headers-handwritten.h | 25 ++++---- test/regression/issue/12548.test.ts | 76 +++++++++++++++++++++++ 4 files changed, 98 insertions(+), 15 deletions(-) create mode 100644 test/regression/issue/12548.test.ts diff --git a/src/bun.js/ModuleLoader.zig b/src/bun.js/ModuleLoader.zig index 378a6bb5c2..c40da5ff99 100644 --- a/src/bun.js/ModuleLoader.zig +++ b/src/bun.js/ModuleLoader.zig @@ -1947,7 +1947,8 @@ export fn Bun__transpileVirtualModule( ) bool { jsc.markBinding(@src()); const jsc_vm = globalObject.bunVM(); - bun.assert(jsc_vm.plugin_runner != null); + // Plugin runner is not required for virtual modules created via build.module() + // bun.assert(jsc_vm.plugin_runner != null); var specifier_slice = specifier_ptr.toUTF8(jsc_vm.allocator); const specifier = specifier_slice.slice(); diff --git a/src/bun.js/bindings/BunPlugin.cpp b/src/bun.js/bindings/BunPlugin.cpp index 3ca7f114f4..c0f830030a 100644 --- a/src/bun.js/bindings/BunPlugin.cpp +++ b/src/bun.js/bindings/BunPlugin.cpp @@ -150,9 +150,14 @@ static EncodedJSValue jsFunctionAppendVirtualModulePluginBody(JSC::JSGlobalObjec virtualModules->set(moduleId, JSC::Strong { vm, jsCast(functionValue) }); - global->requireMap()->remove(globalObject, moduleIdValue); + auto* requireMap = global->requireMap(); RETURN_IF_EXCEPTION(scope, {}); - global->esmRegistryMap()->remove(globalObject, moduleIdValue); + requireMap->remove(globalObject, moduleIdValue); + RETURN_IF_EXCEPTION(scope, {}); + + auto* esmRegistry = global->esmRegistryMap(); + RETURN_IF_EXCEPTION(scope, {}); + esmRegistry->remove(globalObject, moduleIdValue); RETURN_IF_EXCEPTION(scope, {}); return JSValue::encode(callframe->thisValue()); diff --git a/src/bun.js/bindings/headers-handwritten.h b/src/bun.js/bindings/headers-handwritten.h index b52e538af1..8d15002abd 100644 --- a/src/bun.js/bindings/headers-handwritten.h +++ b/src/bun.js/bindings/headers-handwritten.h @@ -225,18 +225,19 @@ const JSErrorCode JSErrorCodeUserErrorCode = 254; // Must be kept in sync. typedef uint8_t BunLoaderType; const BunLoaderType BunLoaderTypeNone = 254; -const BunLoaderType BunLoaderTypeJSX = 0; -const BunLoaderType BunLoaderTypeJS = 1; -const BunLoaderType BunLoaderTypeTS = 2; -const BunLoaderType BunLoaderTypeTSX = 3; -const BunLoaderType BunLoaderTypeCSS = 4; -const BunLoaderType BunLoaderTypeFILE = 5; -const BunLoaderType BunLoaderTypeJSON = 6; -const BunLoaderType BunLoaderTypeJSONC = 7; -const BunLoaderType BunLoaderTypeTOML = 8; -const BunLoaderType BunLoaderTypeWASM = 9; -const BunLoaderType BunLoaderTypeNAPI = 10; -const BunLoaderType BunLoaderTypeYAML = 18; +// 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; #pragma mark - Stream diff --git a/test/regression/issue/12548.test.ts b/test/regression/issue/12548.test.ts new file mode 100644 index 0000000000..09874cc53a --- /dev/null +++ b/test/regression/issue/12548.test.ts @@ -0,0 +1,76 @@ +import { expect, test } from "bun:test"; +import { bunEnv, bunExe, tempDir } from "harness"; + +test("issue #12548: TypeScript syntax should work with 'ts' loader in BunPlugin", async () => { + using dir = tempDir("issue-12548", { + "index.js": ` + import plugin from "./plugin.js"; + + Bun.plugin(plugin); + + // This should work with 'ts' loader + console.log(require('virtual-ts-module')); + `, + "plugin.js": ` + export default { + setup(build) { + build.module('virtual-ts-module', () => ({ + contents: "import { type TSchema } from '@sinclair/typebox'; export const test = 'works';", + loader: 'ts', + })); + }, + }; + `, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "index.js"], + env: bunEnv, + cwd: String(dir), + stderr: "pipe", + stdout: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect(exitCode).toBe(0); + expect(stderr).toBe(""); + expect(stdout).toContain('test: "works"'); +}); + +test("issue #12548: TypeScript type imports work with 'ts' loader", async () => { + using dir = tempDir("issue-12548-type-imports", { + "index.js": ` + Bun.plugin({ + setup(build) { + build.module('test-module', () => ({ + contents: \` + import { type TSchema } from '@sinclair/typebox'; + type MyType = { a: number }; + export type { MyType }; + export const value = 42; + \`, + loader: 'ts', + })); + }, + }); + + const mod = require('test-module'); + console.log(JSON.stringify(mod)); + `, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "index.js"], + env: bunEnv, + cwd: String(dir), + stderr: "pipe", + stdout: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect(exitCode).toBe(0); + expect(stderr).toBe(""); + expect(stdout).toContain('{"value":42}'); +});