mirror of
https://github.com/oven-sh/bun
synced 2026-02-09 18:38:55 +00:00
Fix TypeScript syntax not working with 'ts' loader in BunPlugin (#22460)
## 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 <claude-bot@bun.sh> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
This commit is contained in:
@@ -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();
|
||||
|
||||
@@ -150,9 +150,14 @@ static EncodedJSValue jsFunctionAppendVirtualModulePluginBody(JSC::JSGlobalObjec
|
||||
|
||||
virtualModules->set(moduleId, JSC::Strong<JSC::JSObject> { vm, jsCast<JSC::JSObject*>(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());
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
76
test/regression/issue/12548.test.ts
Normal file
76
test/regression/issue/12548.test.ts
Normal file
@@ -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}');
|
||||
});
|
||||
Reference in New Issue
Block a user