From abbe5f933b75ec0a688271f0c7ec1d2091af1afd Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 27 Aug 2026 01:25:04 +0000 Subject: [PATCH 01/11] Check for import statements in a CommonJS file after the import scanner runs The "Cannot use import statement with CommonJS-only features" check ran before to_ast, where the import scanner marks TypeScript imports that are only used as types as unused. A .ts file with such an import next to module.exports was rejected by require() and, since #40568, by import() as well, although the printed output has no import statement at all. Move the check after to_ast so it skips the elided imports. Make the parser return SyntaxError when errors were logged in this last phase, like the parse and visit phases already do, and drop the now redundant log check in RuntimeTranspilerStore. --- src/js_parser/parse/parse_entry.rs | 174 ++++++++++++------------ src/jsc/RuntimeTranspilerStore.rs | 6 - test/cli/run/transpiler-cache.test.ts | 22 +++ test/js/bun/resolve/build-error.test.ts | 42 ++++++ 4 files changed, 151 insertions(+), 93 deletions(-) diff --git a/src/js_parser/parse/parse_entry.rs b/src/js_parser/parse/parse_entry.rs index 776c25011ac7..d4dae2e7e63f 100644 --- a/src/js_parser/parse/parse_entry.rs +++ b/src/js_parser/parse/parse_entry.rs @@ -1555,6 +1555,10 @@ impl<'a> Parser<'a> { p.symbols.as_slice()[p.module_ref.inner_index() as usize].use_count_estimate > 0; let mut wrap_mode: WrapMode = WrapMode::None; + // An `import` statement cannot be printed inside Bun's CommonJS wrapper. + // Checked after `to_ast`, once the import scanner has marked + // TypeScript type-only imports as unused. + let mut reject_import_statements = false; if p.is_deoptimized_commonjs() { exports_kind = js_ast::ExportsKind::Cjs; @@ -1565,87 +1569,7 @@ impl<'a> Parser<'a> { exports_kind = js_ast::ExportsKind::Cjs; if p.options.features.commonjs_at_runtime { wrap_mode = WrapMode::BunCommonjs; - - let import_record: Option<&ImportRecord> = 'brk: { - for import_record in p.import_records.items() { - if import_record.flags.intersects( - ImportRecordFlags::IS_INTERNAL | ImportRecordFlags::IS_UNUSED, - ) { - continue; - } - if import_record.kind == bun_ast::ImportKind::Stmt { - break 'brk Some(import_record); - } - } - - None - }; - - // make it an error to use an import statement with a commonjs exports usage - if let Some(record) = import_record { - // find the usage of the export symbol - - let mut notes = BumpVec::::new_in(p.arena); - - notes.push(bun_ast::Data { - text: { - use std::io::Write; - let mut v = Vec::::new(); - let _ = write!( - &mut v, - "Try require({}) instead", - bun_core::fmt::QuotedFormatter { - text: record.path.text - } - ); - std::borrow::Cow::Owned(v) - }, - ..Default::default() - }); - - if uses_module_ref { - notes.push(bun_ast::Data { - text: std::borrow::Cow::Borrowed( - b"This file is CommonJS because 'module' was used", - ), - ..Default::default() - }); - } - - if uses_exports_ref { - notes.push(bun_ast::Data { - text: std::borrow::Cow::Borrowed( - b"This file is CommonJS because 'exports' was used", - ), - ..Default::default() - }); - } - - if p.has_top_level_return { - notes.push(bun_ast::Data { - text: std::borrow::Cow::Borrowed( - b"This file is CommonJS because top-level return was used", - ), - ..Default::default() - }); - } - - if p.has_with_scope { - notes.push(bun_ast::Data { - text: std::borrow::Cow::Borrowed( - b"This file is CommonJS because a \"with\" statement is used", - ), - ..Default::default() - }); - } - - p.log().add_range_error_with_notes( - Some(p.source), - record.range, - b"Cannot use import statement with CommonJS-only features".as_slice(), - notes.into_iter().collect::>().into_boxed_slice(), - ); - } + reject_import_statements = true; } } else { match p.options.module_type { @@ -2232,12 +2156,88 @@ impl<'a> Parser<'a> { } } - Ok(crate::Result::Ast(p.to_ast( - &mut parts, - exports_kind, - wrap_mode, - hashbang, - )?)) + let ast = p.to_ast(&mut parts, exports_kind, wrap_mode, hashbang)?; + + if reject_import_statements { + let import_record: Option<&ImportRecord> = + ast.import_records.as_slice().iter().find(|import_record| { + !import_record + .flags + .intersects(ImportRecordFlags::IS_INTERNAL | ImportRecordFlags::IS_UNUSED) + && import_record.kind == bun_ast::ImportKind::Stmt + }); + + if let Some(record) = import_record { + let mut notes = BumpVec::::new_in(p.arena); + + notes.push(bun_ast::Data { + text: { + use std::io::Write; + let mut v = Vec::::new(); + let _ = write!( + &mut v, + "Try require({}) instead", + bun_core::fmt::QuotedFormatter { + text: record.path.text + } + ); + std::borrow::Cow::Owned(v) + }, + ..Default::default() + }); + + if uses_module_ref { + notes.push(bun_ast::Data { + text: std::borrow::Cow::Borrowed( + b"This file is CommonJS because 'module' was used", + ), + ..Default::default() + }); + } + + if uses_exports_ref { + notes.push(bun_ast::Data { + text: std::borrow::Cow::Borrowed( + b"This file is CommonJS because 'exports' was used", + ), + ..Default::default() + }); + } + + if p.has_top_level_return { + notes.push(bun_ast::Data { + text: std::borrow::Cow::Borrowed( + b"This file is CommonJS because top-level return was used", + ), + ..Default::default() + }); + } + + if p.has_with_scope { + notes.push(bun_ast::Data { + text: std::borrow::Cow::Borrowed( + b"This file is CommonJS because a \"with\" statement is used", + ), + ..Default::default() + }); + } + + p.log().add_range_error_with_notes( + Some(p.source), + record.range, + b"Cannot use import statement with CommonJS-only features".as_slice(), + notes.into_iter().collect::>().into_boxed_slice(), + ); + } + } + + // Errors logged during `to_ast` (duplicate export names) or just above + // halt here too, like the parse-phase and visit-phase checks. + if p.log().errors > orig_error_count { + return Err(crate::Error::SyntaxError); + } + + Ok(crate::Result::Ast(ast)) } // associated fn (was `&self` reading `self.lexer.source.contents`) diff --git a/src/jsc/RuntimeTranspilerStore.rs b/src/jsc/RuntimeTranspilerStore.rs index e00baa43410a..ad87fcb9dcf1 100644 --- a/src/jsc/RuntimeTranspilerStore.rs +++ b/src/jsc/RuntimeTranspilerStore.rs @@ -926,12 +926,6 @@ impl TranspilerJob { } } - // The parser can log errors and still return an AST. - if transpiler.log().errors > 0 { - self.parse_error = Some(crate::CrateError::ParseError); - return; - } - // SAFETY: leaf scalar field read; see `vm` note above. Inlined // `VirtualMachine::use_isolation_source_provider_cache` to avoid forming // `&VirtualMachine`. diff --git a/test/cli/run/transpiler-cache.test.ts b/test/cli/run/transpiler-cache.test.ts index cf98f79cef3e..96e10bf773a0 100644 --- a/test/cli/run/transpiler-cache.test.ts +++ b/test/cli/run/transpiler-cache.test.ts @@ -81,6 +81,28 @@ describe("transpiler cache", () => { expect(await bunRun(join(temp_dir, "a.js"), env)).toSpawn("a"); expect(!existsSync(cache_dir)).toBeTrue(); }); + test("does not cache a file whose parse logged an error", async () => { + // The parser reports the `import` next to `module.exports` after it has + // built the AST. Nothing may be printed or cached for such a file, or a + // later run would serve the broken output from the cache without the error. + const filler = "\n//" + Buffer.alloc(5 * 1024, "f").toString(); + writeFileSync(join(temp_dir, "dep.js"), `export const x = 1;`); + writeFileSync(join(temp_dir, "mixed.js"), `import { x } from "./dep.js";\nmodule.exports = { x };` + filler); + writeFileSync( + join(temp_dir, "main.js"), + `const out = {}; + try { await import("./mixed.js"); } catch (e) { out.import = [e.name, e.message]; } + try { require("./mixed.js"); } catch (e) { out.require = [e.name, e.message]; } + console.log(JSON.stringify(out));`, + ); + const expected = JSON.stringify({ + import: ["BuildMessage", "Cannot use import statement with CommonJS-only features"], + require: ["BuildMessage", "Cannot use import statement with CommonJS-only features"], + }); + expect(await bunRun(join(temp_dir, "main.js"), env)).toSpawn(expected); + expect(await bunRun(join(temp_dir, "main.js"), env)).toSpawn(expected); + expect(!existsSync(cache_dir)).toBeTrue(); + }); test("it is indeed content addressable", async () => { writeFileSync(join(temp_dir, "a.js"), dummyFile(50 * 1024, "1", "b")); expect(await bunRun(join(temp_dir, "a.js"), env)).toSpawn("b"); diff --git a/test/js/bun/resolve/build-error.test.ts b/test/js/bun/resolve/build-error.test.ts index a3ce3fe77285..29196e88003f 100644 --- a/test/js/bun/resolve/build-error.test.ts +++ b/test/js/bun/resolve/build-error.test.ts @@ -182,6 +182,48 @@ test("import whose transpile log holds a resolve error rejects with a ResolveMes ]); }); +// TypeScript drops an import whose bindings are only used as types. Such a file +// is plain CommonJS after transpilation, so the `import` must not be reported +// as a conflict with `module.exports`. +test.concurrent("a type-only import next to module.exports loads on every path", async () => { + using dir = tempDir("build-error-type-only-import", { + "types.ts": `export interface Foo { x: number }`, + "mixed.ts": `import { Foo } from "./types";\nconst f: Foo = { x: 1 };\nmodule.exports = { f };`, + "main.ts": ` + const viaImport = (await import("./mixed.ts")).default; + const viaRequire = require("./mixed.ts"); + console.log(JSON.stringify({ import: viaImport, require: viaRequire })); + `, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "main.ts"], + env: bunEnv, + cwd: String(dir), + stderr: "pipe", + }); + await using direct = Bun.spawn({ + cmd: [bunExe(), "mixed.ts"], + env: bunEnv, + cwd: String(dir), + stderr: "pipe", + }); + + const [stdout, stderr, exitCode, directStderr, directExitCode] = await Promise.all([ + proc.stdout.text(), + proc.stderr.text(), + proc.exited, + direct.stderr.text(), + direct.exited, + ]); + + if (exitCode !== 0) console.error(stderr); + expect(JSON.parse(stdout)).toEqual({ import: { f: { x: 1 } }, require: { f: { x: 1 } } }); + expect(exitCode).toBe(0); + expect(directStderr).toBe(""); + expect(directExitCode).toBe(0); +}); + test("BuildMessage finalize frees with the same allocator it was created with", async () => { // BuildMessage.create() clones the message with the passed allocator // but finalize() was freeing it with bun.default_allocator and never From 2e81d78cfb85b2dc99dfb67f412d788222e0c153 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 27 Aug 2026 01:31:10 +0000 Subject: [PATCH 02/11] Assert the direct run's stderr before the exit codes --- test/js/bun/resolve/build-error.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/js/bun/resolve/build-error.test.ts b/test/js/bun/resolve/build-error.test.ts index 29196e88003f..c4403d3faadc 100644 --- a/test/js/bun/resolve/build-error.test.ts +++ b/test/js/bun/resolve/build-error.test.ts @@ -219,8 +219,8 @@ test.concurrent("a type-only import next to module.exports loads on every path", if (exitCode !== 0) console.error(stderr); expect(JSON.parse(stdout)).toEqual({ import: { f: { x: 1 } }, require: { f: { x: 1 } } }); - expect(exitCode).toBe(0); expect(directStderr).toBe(""); + expect(exitCode).toBe(0); expect(directExitCode).toBe(0); }); From 731d427cdd25ca675998b38a19ee7b68d794c287 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 27 Aug 2026 01:33:44 +0000 Subject: [PATCH 03/11] Shorten the two comments around the relocated check --- src/js_parser/parse/parse_entry.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/js_parser/parse/parse_entry.rs b/src/js_parser/parse/parse_entry.rs index d4dae2e7e63f..17ebb8b19089 100644 --- a/src/js_parser/parse/parse_entry.rs +++ b/src/js_parser/parse/parse_entry.rs @@ -1555,9 +1555,7 @@ impl<'a> Parser<'a> { p.symbols.as_slice()[p.module_ref.inner_index() as usize].use_count_estimate > 0; let mut wrap_mode: WrapMode = WrapMode::None; - // An `import` statement cannot be printed inside Bun's CommonJS wrapper. - // Checked after `to_ast`, once the import scanner has marked - // TypeScript type-only imports as unused. + // Checked after `to_ast`, which marks TypeScript type-only imports unused. let mut reject_import_statements = false; if p.is_deoptimized_commonjs() { @@ -2231,8 +2229,7 @@ impl<'a> Parser<'a> { } } - // Errors logged during `to_ast` (duplicate export names) or just above - // halt here too, like the parse-phase and visit-phase checks. + // If there were errors during to_ast, also halt here if p.log().errors > orig_error_count { return Err(crate::Error::SyntaxError); } From 2b31691ca8f8fdd2fb6d52edb4832aca2c6c9378 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 27 Aug 2026 01:49:25 +0000 Subject: [PATCH 04/11] Drain the direct run's stdout as well --- test/js/bun/resolve/build-error.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/js/bun/resolve/build-error.test.ts b/test/js/bun/resolve/build-error.test.ts index c4403d3faadc..b6b5c41b9da4 100644 --- a/test/js/bun/resolve/build-error.test.ts +++ b/test/js/bun/resolve/build-error.test.ts @@ -209,16 +209,18 @@ test.concurrent("a type-only import next to module.exports loads on every path", stderr: "pipe", }); - const [stdout, stderr, exitCode, directStderr, directExitCode] = await Promise.all([ + const [stdout, stderr, exitCode, directStdout, directStderr, directExitCode] = await Promise.all([ proc.stdout.text(), proc.stderr.text(), proc.exited, + direct.stdout.text(), direct.stderr.text(), direct.exited, ]); if (exitCode !== 0) console.error(stderr); expect(JSON.parse(stdout)).toEqual({ import: { f: { x: 1 } }, require: { f: { x: 1 } } }); + expect(directStdout).toBe(""); expect(directStderr).toBe(""); expect(exitCode).toBe(0); expect(directExitCode).toBe(0); From da69a47750c3dfe00790316bd32a048a974e31ff Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 27 Aug 2026 02:43:18 +0000 Subject: [PATCH 05/11] Skip parser-generated imports in the CommonJS import check The JSX runtime import is added by the parser with an empty range and without IS_INTERNAL. Reporting it as "Cannot use import statement" with a note to require("react/jsx-dev-runtime") blames the user for an import they did not write. Leave those records out, so a JSX file with module.exports fails the same way it did before. --- src/js_parser/parse/parse_entry.rs | 3 +++ test/js/bun/resolve/build-error.test.ts | 21 +++++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/src/js_parser/parse/parse_entry.rs b/src/js_parser/parse/parse_entry.rs index 17ebb8b19089..cc7ca99b482e 100644 --- a/src/js_parser/parse/parse_entry.rs +++ b/src/js_parser/parse/parse_entry.rs @@ -2157,12 +2157,15 @@ impl<'a> Parser<'a> { let ast = p.to_ast(&mut parts, exports_kind, wrap_mode, hashbang)?; if reject_import_statements { + // An empty range marks a record the parser generated (the JSX runtime + // import). Only an import the user wrote gets this error. let import_record: Option<&ImportRecord> = ast.import_records.as_slice().iter().find(|import_record| { !import_record .flags .intersects(ImportRecordFlags::IS_INTERNAL | ImportRecordFlags::IS_UNUSED) && import_record.kind == bun_ast::ImportKind::Stmt + && !import_record.range.is_empty() }); if let Some(record) = import_record { diff --git a/test/js/bun/resolve/build-error.test.ts b/test/js/bun/resolve/build-error.test.ts index b6b5c41b9da4..09713df22ca0 100644 --- a/test/js/bun/resolve/build-error.test.ts +++ b/test/js/bun/resolve/build-error.test.ts @@ -226,6 +226,27 @@ test.concurrent("a type-only import next to module.exports loads on every path", expect(directExitCode).toBe(0); }); +// The parser adds the JSX runtime import itself. It must not be reported as +// an import statement the user should replace with require(). +test.concurrent("JSX next to module.exports is not blamed on a runtime import", async () => { + using dir = tempDir("build-error-jsx-cjs", { + "mixed.tsx": `const el =
;\nmodule.exports = { el };`, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "mixed.tsx"], + env: bunEnv, + cwd: String(dir), + stderr: "pipe", + }); + + const [stdout, stderr] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect(stdout).toBe(""); + expect(stderr).not.toContain("Cannot use import statement with CommonJS-only features"); + expect(stderr).not.toContain("Try require("); +}); + test("BuildMessage finalize frees with the same allocator it was created with", async () => { // BuildMessage.create() clones the message with the passed allocator // but finalize() was freeing it with bun.default_allocator and never From b9dfae0c73703388d10d88db34caf85666a980eb Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 27 Aug 2026 02:45:24 +0000 Subject: [PATCH 06/11] One-line comment for the parser-generated record skip --- src/js_parser/parse/parse_entry.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/js_parser/parse/parse_entry.rs b/src/js_parser/parse/parse_entry.rs index cc7ca99b482e..9693e8dc961d 100644 --- a/src/js_parser/parse/parse_entry.rs +++ b/src/js_parser/parse/parse_entry.rs @@ -2157,8 +2157,7 @@ impl<'a> Parser<'a> { let ast = p.to_ast(&mut parts, exports_kind, wrap_mode, hashbang)?; if reject_import_statements { - // An empty range marks a record the parser generated (the JSX runtime - // import). Only an import the user wrote gets this error. + // An empty range marks a parser-generated record, like the JSX runtime import. let import_record: Option<&ImportRecord> = ast.import_records.as_slice().iter().find(|import_record| { !import_record From cd89feff49b41c141d62ff6dc06fc15c6d69beb2 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 27 Aug 2026 05:36:50 +0000 Subject: [PATCH 07/11] Count lexer errors from the first token against the parse Parser::init primes the lexer with one next() call, and _parse took its error baseline only after that. An error the lexer logged for the first token (an identifier spelled with escapes, such as \u0030foo, or an out-of-range Unicode escape) was invisible to the three halt checks, so the parse returned an AST with an error in the log. require() caught it through the log check in the sync loader. import() did not, and either executed the file or handed the printed output to JSC and cached it. Take the baseline in Parser::init before the priming next() and carry it on the Parser. A debug assertion in cache::JavaScript::parse pins that an AST never comes back with a logged error. --- src/bundler/cache.rs | 7 ++++- src/js_parser/parse/parse_entry.rs | 9 ++++-- src/js_parser/parser.rs | 1 + test/cli/run/transpiler-cache.test.ts | 18 ++++++++---- test/js/bun/resolve/build-error.test.ts | 37 +++++++++++++++++++++++++ 5 files changed, 63 insertions(+), 9 deletions(-) diff --git a/src/bundler/cache.rs b/src/bundler/cache.rs index e500a858c7aa..64f4bbe5f17c 100644 --- a/src/bundler/cache.rs +++ b/src/bundler/cache.rs @@ -87,7 +87,12 @@ impl JavaScript { }; let result = match parser.parse() { - Ok(r) => r, + Ok(r) => { + // Every phase of the parse halts on a logged error, so callers + // may run or cache an AST without checking the log. + debug_assert!(!matches!(r, js_parser::Result::Ast(_)) || temp_log.errors == 0); + r + } Err(err) => { // `Parser::parse` consumes `self`, so `parser` is gone in this // arm. The `&'a mut temp_log` it held is released, so read diff --git a/src/js_parser/parse/parse_entry.rs b/src/js_parser/parse/parse_entry.rs index 9693e8dc961d..183875cb3990 100644 --- a/src/js_parser/parse/parse_entry.rs +++ b/src/js_parser/parse/parse_entry.rs @@ -62,6 +62,9 @@ pub struct Parser<'a> { pub(crate) source: &'a bun_ast::Source, pub(crate) define: &'a Define, pub(crate) bump: &'a Arena, + /// `log.errors` before the priming `lexer.next()` in `init`. An error the + /// lexer logs for the first token must count against the parse too. + pub(crate) orig_error_count: u32, } pub struct Options<'a> { @@ -314,6 +317,7 @@ impl<'a> Parser<'a> { bump: &'a Arena, ) -> Result, Error> { source.check_parseable_len(log, "File")?; + let orig_error_count = log.errors; let mut lexer = js_lexer::Lexer::init_without_reading(log, source, bump); // Must be set before the priming `next()` so leading comments are seen. lexer.track_comments = options.features.minify_identifiers; @@ -330,6 +334,7 @@ impl<'a> Parser<'a> { define, source, log: log_ptr, + orig_error_count, }) } } @@ -741,11 +746,9 @@ impl<'a> Parser<'a> { source, define, bump, + orig_error_count, } = self; - // `lexer.log` aliases `log`; route through the centralised - // `Lexer::log()` accessor so this site stays safe. - let orig_error_count = lexer.log().errors; // `P.log` and `Lexer.log` are both `NonNull` (see P.rs / lexer.rs // field docs), so handing the same raw pointer to both is defined — // no `&mut` is materialized. diff --git a/src/js_parser/parser.rs b/src/js_parser/parser.rs index 20db6ddf9d31..408deeeb45c0 100644 --- a/src/js_parser/parser.rs +++ b/src/js_parser/parser.rs @@ -1688,6 +1688,7 @@ pub fn new_lazy_export_ast_impl<'bump>( define, source, log: log_ptr, + orig_error_count: 0, }; let result = match parser.to_lazy_export_ast(expr, runtime_api_call, symbols) { Ok(r) => r, diff --git a/test/cli/run/transpiler-cache.test.ts b/test/cli/run/transpiler-cache.test.ts index 96e10bf773a0..995a47f81783 100644 --- a/test/cli/run/transpiler-cache.test.ts +++ b/test/cli/run/transpiler-cache.test.ts @@ -83,21 +83,29 @@ describe("transpiler cache", () => { }); test("does not cache a file whose parse logged an error", async () => { // The parser reports the `import` next to `module.exports` after it has - // built the AST. Nothing may be printed or cached for such a file, or a + // built the AST, and the lexer reports `0foo` while the parser is being + // constructed. Nothing may be printed or cached for such a file, or a // later run would serve the broken output from the cache without the error. const filler = "\n//" + Buffer.alloc(5 * 1024, "f").toString(); writeFileSync(join(temp_dir, "dep.js"), `export const x = 1;`); writeFileSync(join(temp_dir, "mixed.js"), `import { x } from "./dep.js";\nmodule.exports = { x };` + filler); + writeFileSync(join(temp_dir, "first.js"), `\\u0030foo = 1;` + filler); writeFileSync( join(temp_dir, "main.js"), `const out = {}; - try { await import("./mixed.js"); } catch (e) { out.import = [e.name, e.message]; } - try { require("./mixed.js"); } catch (e) { out.require = [e.name, e.message]; } + for (const file of ["./mixed.js", "./first.js"]) { + try { await import(file); } catch (e) { out["import " + file] = [e.name, e.message]; } + try { require(file); } catch (e) { out["require " + file] = [e.name, e.message]; } + } console.log(JSON.stringify(out));`, ); + const mixed = ["BuildMessage", "Cannot use import statement with CommonJS-only features"]; + const first = ["BuildMessage", 'Invalid identifier: "0foo"']; const expected = JSON.stringify({ - import: ["BuildMessage", "Cannot use import statement with CommonJS-only features"], - require: ["BuildMessage", "Cannot use import statement with CommonJS-only features"], + "import ./mixed.js": mixed, + "require ./mixed.js": mixed, + "import ./first.js": first, + "require ./first.js": first, }); expect(await bunRun(join(temp_dir, "main.js"), env)).toSpawn(expected); expect(await bunRun(join(temp_dir, "main.js"), env)).toSpawn(expected); diff --git a/test/js/bun/resolve/build-error.test.ts b/test/js/bun/resolve/build-error.test.ts index 09713df22ca0..63ffd5ca9f97 100644 --- a/test/js/bun/resolve/build-error.test.ts +++ b/test/js/bun/resolve/build-error.test.ts @@ -247,6 +247,43 @@ test.concurrent("JSX next to module.exports is not blamed on a runtime import", expect(stderr).not.toContain("Try require("); }); +// The lexer reads the first token while the parser is constructed. An error it +// logs there still has to fail the parse, on both load paths. +test.concurrent("a lexer error on the first token rejects import() and require()", async () => { + using dir = tempDir("build-error-first-token", { + // `\u0030foo` spells the identifier `0foo`. + "bad.js": `\\u0030foo = 1;\nconsole.log("loaded");`, + "main.js": ` + const out = {}; + try { + await import("./bad.js"); + } catch (e) { + out.import = [e.name, e.message]; + } + try { + require("./bad.js"); + } catch (e) { + out.require = [e.name, e.message]; + } + console.log(JSON.stringify(out)); + `, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "main.js"], + env: bunEnv, + cwd: String(dir), + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + if (exitCode !== 0) console.error(stderr); + const expected = ["BuildMessage", 'Invalid identifier: "0foo"']; + expect(JSON.parse(stdout)).toEqual({ import: expected, require: expected }); + expect(exitCode).toBe(0); +}); + test("BuildMessage finalize frees with the same allocator it was created with", async () => { // BuildMessage.create() clones the message with the passed allocator // but finalize() was freeing it with bun.default_allocator and never From a7cbca2fb3dcb5f621f4f9c565cd828588133807 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 27 Aug 2026 05:40:35 +0000 Subject: [PATCH 08/11] One-line comments for the error baseline and the assertion --- src/bundler/cache.rs | 3 +-- src/js_parser/parse/parse_entry.rs | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/bundler/cache.rs b/src/bundler/cache.rs index 64f4bbe5f17c..9c0df69a7a06 100644 --- a/src/bundler/cache.rs +++ b/src/bundler/cache.rs @@ -88,8 +88,7 @@ impl JavaScript { let result = match parser.parse() { Ok(r) => { - // Every phase of the parse halts on a logged error, so callers - // may run or cache an AST without checking the log. + // The parser halts on every logged error, so an AST never comes with one. debug_assert!(!matches!(r, js_parser::Result::Ast(_)) || temp_log.errors == 0); r } diff --git a/src/js_parser/parse/parse_entry.rs b/src/js_parser/parse/parse_entry.rs index 183875cb3990..b383be22b9bb 100644 --- a/src/js_parser/parse/parse_entry.rs +++ b/src/js_parser/parse/parse_entry.rs @@ -62,8 +62,7 @@ pub struct Parser<'a> { pub(crate) source: &'a bun_ast::Source, pub(crate) define: &'a Define, pub(crate) bump: &'a Arena, - /// `log.errors` before the priming `lexer.next()` in `init`. An error the - /// lexer logs for the first token must count against the parse too. + /// `log.errors` before the priming `lexer.next()` in `init`. pub(crate) orig_error_count: u32, } From eea7b4e541b88cc1973dfdc863580dae081ea664 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 27 Aug 2026 05:45:25 +0000 Subject: [PATCH 09/11] Assert the JSX file's exit code --- test/js/bun/resolve/build-error.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/js/bun/resolve/build-error.test.ts b/test/js/bun/resolve/build-error.test.ts index 63ffd5ca9f97..6f4738a02a49 100644 --- a/test/js/bun/resolve/build-error.test.ts +++ b/test/js/bun/resolve/build-error.test.ts @@ -240,11 +240,14 @@ test.concurrent("JSX next to module.exports is not blamed on a runtime import", stderr: "pipe", }); - const [stdout, stderr] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + // The file still fails to load: the generated import is printed inside the + // CommonJS wrapper, and JSC rejects that. The error must not name it. expect(stdout).toBe(""); expect(stderr).not.toContain("Cannot use import statement with CommonJS-only features"); expect(stderr).not.toContain("Try require("); + expect(exitCode).toBe(1); }); // The lexer reads the first token while the parser is constructed. An error it From 027854bdd1ddbfae7ec330cb67cf06ba1ade78d2 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 27 Aug 2026 06:53:35 +0000 Subject: [PATCH 10/11] Halt on a first-token lexer error before the pragma and cache returns The "// @bun" pragma and the runtime transpiler cache return from _parse before the first halt check, so an error logged for the first token could still come back with an Ok result on those paths. Check for it right after the hashbang, so every Ok result carries no logged error. --- src/bundler/cache.rs | 4 ++-- src/js_parser/parse/parse_entry.rs | 5 ++++ test/js/bun/resolve/build-error.test.ts | 31 ++++++++++++++++--------- 3 files changed, 27 insertions(+), 13 deletions(-) diff --git a/src/bundler/cache.rs b/src/bundler/cache.rs index 9c0df69a7a06..fb867d899c3c 100644 --- a/src/bundler/cache.rs +++ b/src/bundler/cache.rs @@ -88,8 +88,8 @@ impl JavaScript { let result = match parser.parse() { Ok(r) => { - // The parser halts on every logged error, so an AST never comes with one. - debug_assert!(!matches!(r, js_parser::Result::Ast(_)) || temp_log.errors == 0); + // The parser halts on every logged error. + debug_assert_eq!(temp_log.errors, 0); r } Err(err) => { diff --git a/src/js_parser/parse/parse_entry.rs b/src/js_parser/parse/parse_entry.rs index b383be22b9bb..822e310e2cdb 100644 --- a/src/js_parser/parse/parse_entry.rs +++ b/src/js_parser/parse/parse_entry.rs @@ -777,6 +777,11 @@ impl<'a> Parser<'a> { p.lexer.next()?; } + // The first token may already have logged an error; halt before the early returns below. + if p.log().errors > orig_error_count { + return Err(crate::Error::SyntaxError); + } + // Detect a leading "// @bun" pragma if p.options.features.dont_bundle_twice { if let Some(pragma) = Self::has_bun_pragma(&source.contents, !hashbang.is_empty()) { diff --git a/test/js/bun/resolve/build-error.test.ts b/test/js/bun/resolve/build-error.test.ts index 6f4738a02a49..26b687725e26 100644 --- a/test/js/bun/resolve/build-error.test.ts +++ b/test/js/bun/resolve/build-error.test.ts @@ -251,22 +251,26 @@ test.concurrent("JSX next to module.exports is not blamed on a runtime import", }); // The lexer reads the first token while the parser is constructed. An error it -// logs there still has to fail the parse, on both load paths. +// logs there still has to fail the parse, on both load paths, also when the +// `// @bun` pragma makes the parser hand the file over without parsing it. test.concurrent("a lexer error on the first token rejects import() and require()", async () => { using dir = tempDir("build-error-first-token", { // `\u0030foo` spells the identifier `0foo`. "bad.js": `\\u0030foo = 1;\nconsole.log("loaded");`, + "prebundled.js": `// @bun\n\\u0030foo = 1;\nconsole.log("loaded");`, "main.js": ` const out = {}; - try { - await import("./bad.js"); - } catch (e) { - out.import = [e.name, e.message]; - } - try { - require("./bad.js"); - } catch (e) { - out.require = [e.name, e.message]; + for (const file of ["./bad.js", "./prebundled.js"]) { + try { + await import(file); + } catch (e) { + out["import " + file] = [e.name, e.message]; + } + try { + require(file); + } catch (e) { + out["require " + file] = [e.name, e.message]; + } } console.log(JSON.stringify(out)); `, @@ -283,7 +287,12 @@ test.concurrent("a lexer error on the first token rejects import() and require() if (exitCode !== 0) console.error(stderr); const expected = ["BuildMessage", 'Invalid identifier: "0foo"']; - expect(JSON.parse(stdout)).toEqual({ import: expected, require: expected }); + expect(JSON.parse(stdout)).toEqual({ + "import ./bad.js": expected, + "require ./bad.js": expected, + "import ./prebundled.js": expected, + "require ./prebundled.js": expected, + }); expect(exitCode).toBe(0); }); From 54ebaa235080762648beb1d81238ba775b950a66 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 27 Aug 2026 07:00:11 +0000 Subject: [PATCH 11/11] Assert empty stderr on the successful runs in the new tests --- test/js/bun/resolve/build-error.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/js/bun/resolve/build-error.test.ts b/test/js/bun/resolve/build-error.test.ts index 26b687725e26..18e4d00a6b2a 100644 --- a/test/js/bun/resolve/build-error.test.ts +++ b/test/js/bun/resolve/build-error.test.ts @@ -218,7 +218,7 @@ test.concurrent("a type-only import next to module.exports loads on every path", direct.exited, ]); - if (exitCode !== 0) console.error(stderr); + expect(stderr).toBe(""); expect(JSON.parse(stdout)).toEqual({ import: { f: { x: 1 } }, require: { f: { x: 1 } } }); expect(directStdout).toBe(""); expect(directStderr).toBe(""); @@ -285,7 +285,7 @@ test.concurrent("a lexer error on the first token rejects import() and require() const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - if (exitCode !== 0) console.error(stderr); + expect(stderr).toBe(""); const expected = ["BuildMessage", 'Invalid identifier: "0foo"']; expect(JSON.parse(stdout)).toEqual({ "import ./bad.js": expected,