diff --git a/src/js_parser/parse/parse_entry.rs b/src/js_parser/parse/parse_entry.rs index 713900cde056..27e4cedde698 100644 --- a/src/js_parser/parse/parse_entry.rs +++ b/src/js_parser/parse/parse_entry.rs @@ -259,6 +259,10 @@ impl<'a> Options<'a> { hasher.update(b"no_dce"); } + // `module_type` changes the parse result for byte-identical sources + // (e.g. `export` in an explicit-CommonJS file is a hard error). + hasher.update(&[self.module_type as u8]); + self.features.hash_for_runtime_transpiler(hasher); } @@ -1626,6 +1630,57 @@ impl<'a> Parser<'a> { exports_kind = js_ast::ExportsKind::Cjs; } else if p.esm_export_keyword.len > 0 || p.top_level_await_keyword.len > 0 { exports_kind = js_ast::ExportsKind::Esm; + // .cjs / "type":"commonjs" are authoritative: Node rejects `export` + // and top-level `await` in those files as a SyntaxError. + // Runtime-only; the bundler has its own format resolution. + // TypeScript excluded: `export` in a CommonJS-typed .ts/.cts is + // idiomatic (tsc compiles it to `exports.x = ...`), so rejecting it + // would break the normal way to author CJS in TS. + if p.options.module_type == options::ModuleType::Cjs + && !p.options.bundle + && !p.options.ts + && p.options.features.commonjs_at_runtime + { + let (range, what): (bun_ast::Range, &'static [u8]) = if p.esm_export_keyword.len > 0 + { + ( + p.esm_export_keyword, + b"Cannot use 'export' in a CommonJS module", + ) + } else { + ( + p.top_level_await_keyword, + b"Cannot use top-level 'await' in a CommonJS module", + ) + }; + let ext = p.source.path.name().ext; + let (why, how): (&'static [u8], &'static [u8]) = if ext == b".cjs" { + ( + b"This file is CommonJS because of its \".cjs\" extension", + b"To use ES module syntax, change the file extension to '.mjs'", + ) + } else { + ( + b"This file is CommonJS because the nearest package.json sets \"type\": \"commonjs\"", + b"To use ES module syntax, change the file extension to '.mjs' or set \"type\": \"module\" in package.json", + ) + }; + p.log().add_range_error_with_notes( + Some(p.source), + range, + what, + Box::new([ + bun_ast::Data { + text: std::borrow::Cow::Borrowed(why), + ..Default::default() + }, + bun_ast::Data { + text: std::borrow::Cow::Borrowed(how), + ..Default::default() + }, + ]), + ); + } } else if uses_exports_ref || uses_module_ref || p.has_top_level_return || p.has_with_scope { exports_kind = js_ast::ExportsKind::Cjs; @@ -1724,7 +1779,12 @@ impl<'a> Parser<'a> { && !p.has_top_level_return && !p.has_with_scope ); - // Use ESM if the file has ES module syntax (import) + // Use ESM if the file has ES module syntax (import). + // Node rejects `import` in a .cjs/"type":"commonjs" file + // the same way it rejects `export`/TLA (handled above), + // but import-only in explicit-CJS is deliberately still + // allowed here: it is far more prevalent in the wild than + // export/TLA, and rejecting it would be much more breaking. exports_kind = if p.has_es_module_syntax { js_ast::ExportsKind::Esm } else { diff --git a/src/jsc/RuntimeTranspilerCache.rs b/src/jsc/RuntimeTranspilerCache.rs index 175f9ffe8dd3..b99f54de71d8 100644 --- a/src/jsc/RuntimeTranspilerCache.rs +++ b/src/jsc/RuntimeTranspilerCache.rs @@ -43,7 +43,9 @@ bun_core::declare_scope!(cache, visible); /// path reinstates the bug for any previously-cached TLA module (#30887). /// Version 23: `jsx.runtime`/`jsx.development` participate in the features hash, /// and tsconfig `"jsx": "react-jsx"` now emits the production runtime (#4227). -const EXPECTED_VERSION: u32 = 23; +/// Version 24: `module_type` is part of the features hash; `export`/TLA in an +/// explicit-CommonJS file is now a parse error. +const EXPECTED_VERSION: u32 = 24; /// Source files smaller than this are not written to / read from the on-disk /// transpiler cache. Originally 50 KiB, which excluded almost every file in a diff --git a/src/jsc/RuntimeTranspilerStore.rs b/src/jsc/RuntimeTranspilerStore.rs index 8ba138599be3..2e5492939f67 100644 --- a/src/jsc/RuntimeTranspilerStore.rs +++ b/src/jsc/RuntimeTranspilerStore.rs @@ -309,6 +309,7 @@ impl RuntimeTranspilerStore { path: &Fs::Path<'_>, referrer: String, loader: Loader, + module_type: ModuleType, package_json: Option<&PackageJSON>, ) -> *mut c_void { // The path text is heap-duplicated here and freed in `reset_for_pool` via @@ -352,6 +353,7 @@ impl RuntimeTranspilerStore { vm, log: bun_ast::Log::init(), loader, + module_type, promise: StrongOptional::create(JSValue::from_cell(promise), global_object), poll_ref: KeepAlive::default(), fetcher: Fetcher::File, @@ -401,6 +403,7 @@ pub struct TranspilerJob { pub non_threadsafe_input_specifier: OwnedString, pub non_threadsafe_referrer: OwnedString, pub loader: Loader, + pub module_type: ModuleType, pub promise: StrongOptional, // Note: struct is stored in a HiveArray and crosses to a worker thread; // raw pointers/BackRefs are used (BACKREF — VM owns the @@ -816,11 +819,7 @@ impl TranspilerJob { && vm_main_hash == hash && strings::eql_long(vm_main, path.text, false); - let module_type: ModuleType = match this_tag { - ResolvedSourceTag::PackageJsonTypeCommonjs => ModuleType::Cjs, - ResolvedSourceTag::PackageJsonTypeModule => ModuleType::Esm, - _ => ModuleType::Unknown, - }; + let module_type = self.module_type; let mut parse_options = ParseOptions { arena: &arena, @@ -960,6 +959,16 @@ impl TranspilerJob { } } + // The parser may emit errors into the log without `parse_maybe` itself + // returning `None` (e.g. "Cannot use 'export' in a CommonJS module"). + // The sync path in `transpile_source_code_inner` checks this after the + // parse; do the same here so those errors surface instead of silently + // executing the output. + 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/src/runtime/jsc_hooks.rs b/src/runtime/jsc_hooks.rs index 98b88bc1ee62..43d5e72d0555 100644 --- a/src/runtime/jsc_hooks.rs +++ b/src/runtime/jsc_hooks.rs @@ -3909,6 +3909,9 @@ struct LoaderResult<'a> { specifier: &'a [u8], /// Always `None` for non-JS-like loaders (not needed there). package_json: Option<&'a bun_resolver::package_json::PackageJSON>, + /// `"type"` from the nearest ancestor package.json, including nameless ones + /// that `enclosing_package_json` skips. `Unknown` for non-JS-like loaders. + nearest_module_type: ModuleType, } /// `options.getLoaderAndVirtualSource` — high-tier body. @@ -4032,15 +4035,34 @@ unsafe fn get_loader_and_virtual_source<'a>( // package.json sniff for `.js`/`.ts` module-type. let dir = path.name().dir; let is_js_like = loader.map(|l| l.is_java_script_like()).unwrap_or(true); - let package_json = if is_js_like && bun_paths::is_absolute(dir) { + let (package_json, nearest_module_type) = if is_js_like && bun_paths::is_absolute(dir) { // SAFETY: per fn contract — `transpiler.resolver` is a value field of // the VM; `read_dir_info` is re-entrant on the JS thread. match unsafe { (*jsc_vm).transpiler.resolver.read_dir_info(dir) } { - Ok(Some(dir_info)) => dir_info.package_json().or(dir_info.enclosing_package_json), - _ => None, + Ok(Some(dir_info)) => { + // `enclosing_package_json` skips nameless package.json files, but + // Node's "type" determination uses the nearest one regardless of + // name (e.g. puppeteer `lib/esm/package.json` = {"type":"module"} + // under a root `{"type":"commonjs"}`). + let mut walk = Some(dir_info); + let nearest_module_type = loop { + match walk { + Some(di) => match di.package_json() { + Some(pj) => break pj.module_type, + None => walk = di.get_parent(), + }, + None => break ModuleType::Unknown, + } + }; + ( + dir_info.package_json().or(dir_info.enclosing_package_json), + nearest_module_type, + ) + } + _ => (None, ModuleType::Unknown), } } else { - None + (None, ModuleType::Unknown) }; Ok(LoaderResult { @@ -4050,6 +4072,7 @@ unsafe fn get_loader_and_virtual_source<'a>( is_main, specifier, package_json, + nearest_module_type, }) } @@ -4269,11 +4292,8 @@ unsafe fn transpile_file( } // regex /\.[jt]s$/ if ext.len() == b".ts".len() && (ext == b".js" || ext == b".ts") { - // Use the package.json module type if it exists. - break 'brk lr - .package_json - .map(|pkg| pkg.module_type) - .unwrap_or(ModuleType::Unknown); + // Use the nearest package.json's "type" (including nameless ones). + break 'brk lr.nearest_module_type; } // For JSX/TSX and other extensions, let the file contents decide. ModuleType::Unknown @@ -4329,6 +4349,7 @@ unsafe fn transpile_file( &lr.path, (*referrer).dupe_ref(), concurrent_loader, + module_type, lr.package_json, ) }; diff --git a/test/cli/run/run-cjs.test.ts b/test/cli/run/run-cjs.test.ts index ebf49dba3f5a..a247a225e576 100644 --- a/test/cli/run/run-cjs.test.ts +++ b/test/cli/run/run-cjs.test.ts @@ -1,6 +1,6 @@ import { describe, expect, test } from "bun:test"; import { mkdirSync } from "fs"; -import { bunEnv, bunExe, tmpdirSync } from "harness"; +import { bunEnv, bunExe, tempDir, tmpdirSync } from "harness"; import { join } from "path"; describe.concurrent("run-cjs", () => { @@ -18,3 +18,189 @@ describe.concurrent("run-cjs", () => { expect(stdout).toEqual("hello world\n"); }); }); + +describe.concurrent("explicit CommonJS module type rejects ESM-only syntax", () => { + async function run(cwd: string, entry: string) { + await using proc = Bun.spawn({ + cmd: [bunExe(), entry], + cwd, + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + return { stdout, stderr, exitCode }; + } + + const exportBody = `export const x = 7;\nconsole.log("ran");\n`; + const tlaBody = `await Promise.resolve();\nconsole.log("ran");\n`; + + test("export in .cjs is an error", async () => { + using dir = tempDir("cjs-export", { "t.cjs": exportBody }); + const { stdout, stderr, exitCode } = await run(String(dir), "t.cjs"); + expect(stderr).toContain("Cannot use 'export' in a CommonJS module"); + expect(stderr).toContain('This file is CommonJS because of its ".cjs" extension'); + expect({ stdout, exitCode }).toEqual({ stdout: "", exitCode: 1 }); + }); + + test('export in .js under "type":"commonjs" is an error', async () => { + using dir = tempDir("tc-export", { + "package.json": `{"type":"commonjs"}`, + "t.js": exportBody, + }); + const { stdout, stderr, exitCode } = await run(String(dir), "t.js"); + expect(stderr).toContain("Cannot use 'export' in a CommonJS module"); + expect(stderr).toContain('the nearest package.json sets "type": "commonjs"'); + expect({ stdout, exitCode }).toEqual({ stdout: "", exitCode: 1 }); + }); + + test("top-level await in .cjs is an error", async () => { + using dir = tempDir("cjs-tla", { "t.cjs": tlaBody }); + const { stdout, stderr, exitCode } = await run(String(dir), "t.cjs"); + expect(stderr).toContain("Cannot use top-level 'await' in a CommonJS module"); + expect(stderr).toContain('This file is CommonJS because of its ".cjs" extension'); + expect({ stdout, exitCode }).toEqual({ stdout: "", exitCode: 1 }); + }); + + test('top-level await in .js under "type":"commonjs" is an error', async () => { + using dir = tempDir("tc-tla", { + "package.json": `{"type":"commonjs"}`, + "t.js": tlaBody, + }); + const { stdout, stderr, exitCode } = await run(String(dir), "t.js"); + expect(stderr).toContain("Cannot use top-level 'await' in a CommonJS module"); + expect(stderr).toContain('the nearest package.json sets "type": "commonjs"'); + expect({ stdout, exitCode }).toEqual({ stdout: "", exitCode: 1 }); + }); + + test('.js under "type":"commonjs" with export rejects even when imported from .mjs', async () => { + using dir = tempDir("tc-imported", { + "pkg/package.json": `{"name":"p","type":"commonjs"}`, + "pkg/t.js": exportBody, + "p.mjs": `import { x } from "./pkg/t.js"; console.log("named", x);\n`, + }); + const { stdout, stderr, exitCode } = await run(String(dir), "p.mjs"); + expect(stderr).toContain("Cannot use 'export' in a CommonJS module"); + expect({ stdout, exitCode }).toEqual({ stdout: "", exitCode: 1 }); + }); + + test(".cjs with export rejects when loaded via dynamic import()", async () => { + using dir = tempDir("cjs-dyn", { + "t.cjs": exportBody, + "entry.mjs": `await import("./t.cjs");\n`, + }); + const { stdout, stderr, exitCode } = await run(String(dir), "entry.mjs"); + expect(stderr).toContain("Cannot use 'export' in a CommonJS module"); + expect({ stdout, exitCode }).toEqual({ stdout: "", exitCode: 1 }); + }); + + test(".cjs with export rejects when loaded via require()", async () => { + using dir = tempDir("cjs-req", { + "t.cjs": exportBody, + "entry.cjs": `require("./t.cjs");\n`, + }); + const { stdout, stderr, exitCode } = await run(String(dir), "entry.cjs"); + expect(stderr).toContain("Cannot use 'export' in a CommonJS module"); + expect({ stdout, exitCode }).toEqual({ stdout: "", exitCode: 1 }); + }); + + // Negative controls: these must keep working. + + test("export in .mjs still runs as ESM", async () => { + using dir = tempDir("mjs-export", { "t.mjs": exportBody }); + const { stdout, stderr, exitCode } = await run(String(dir), "t.mjs"); + expect(stderr).not.toContain("Cannot use 'export' in a CommonJS module"); + expect({ stdout, exitCode }).toEqual({ stdout: "ran\n", exitCode: 0 }); + }); + + test('export in .js under "type":"module" still runs as ESM', async () => { + using dir = tempDir("tm-export", { + "package.json": `{"type":"module"}`, + "t.js": exportBody, + }); + const { stdout, stderr, exitCode } = await run(String(dir), "t.js"); + expect(stderr).not.toContain("Cannot use 'export' in a CommonJS module"); + expect({ stdout, exitCode }).toEqual({ stdout: "ran\n", exitCode: 0 }); + }); + + test("export in .js with no package.json type still runs (ambiguous: content decides)", async () => { + using dir = tempDir("untyped-export", { "t.js": exportBody }); + const { stdout, stderr, exitCode } = await run(String(dir), "t.js"); + expect(stderr).not.toContain("Cannot use 'export' in a CommonJS module"); + expect({ stdout, exitCode }).toEqual({ stdout: "ran\n", exitCode: 0 }); + }); + + test('.mjs inside "type":"commonjs" package still runs as ESM (extension wins)', async () => { + using dir = tempDir("mjs-in-cjs", { + "package.json": `{"type":"commonjs"}`, + "t.mjs": exportBody, + }); + const { stdout, stderr, exitCode } = await run(String(dir), "t.mjs"); + expect(stderr).not.toContain("Cannot use 'export' in a CommonJS module"); + expect({ stdout, exitCode }).toEqual({ stdout: "ran\n", exitCode: 0 }); + }); + + test('nameless nested {"type":"module"} overrides outer {"type":"commonjs"} (dual-package layout)', async () => { + // e.g. puppeteer: root `{"type":"commonjs"}`, `lib/esm/package.json` = `{"type":"module"}`. + using dir = tempDir("nested-esm", { + "package.json": `{"name":"pkg","type":"commonjs"}`, + "lib/esm/package.json": `{"type":"module"}`, + "lib/esm/inner/t.js": exportBody, + "entry.mjs": `const ns = await import("./lib/esm/inner/t.js");\nconsole.log(JSON.stringify({ x: ns.x, hasDefault: "default" in ns }));\n`, + }); + const { stdout, stderr, exitCode } = await run(String(dir), "entry.mjs"); + expect(stderr).not.toContain("Cannot use 'export' in a CommonJS module"); + expect({ stdout, exitCode }).toEqual({ + stdout: `ran\n${JSON.stringify({ x: 7, hasDefault: false })}\n`, + exitCode: 0, + }); + }); + + test('nameless nested {"type":"commonjs"} overrides outer {"type":"module"}', async () => { + using dir = tempDir("nested-cjs", { + "package.json": `{"name":"pkg","type":"module"}`, + "dist/cjs/package.json": `{"type":"commonjs"}`, + "dist/cjs/inner/t.js": exportBody, + "entry.mjs": `await import("./dist/cjs/inner/t.js");\n`, + }); + const { stdout, stderr, exitCode } = await run(String(dir), "entry.mjs"); + expect(stderr).toContain("Cannot use 'export' in a CommonJS module"); + expect(stderr).toContain('the nearest package.json sets "type": "commonjs"'); + expect({ stdout, exitCode }).toEqual({ stdout: "", exitCode: 1 }); + }); + + test("module.exports in .cjs still runs as CJS", async () => { + using dir = tempDir("cjs-modexp", { + "t.cjs": `module.exports = { x: 7 };\nconsole.log("ran");\n`, + }); + const { stdout, stderr, exitCode } = await run(String(dir), "t.cjs"); + expect(stderr).not.toContain("Cannot use 'export' in a CommonJS module"); + expect({ stdout, exitCode }).toEqual({ stdout: "ran\n", exitCode: 0 }); + }); + + // TypeScript is intentionally excluded: `export` in a CommonJS-typed .ts/.cts + // is idiomatic (tsc compiles it to `exports.x = ...`). + test("export in .cts is not an error (TypeScript excluded)", async () => { + using dir = tempDir("cts-export", { "t.cts": exportBody }); + const { stdout, stderr, exitCode } = await run(String(dir), "t.cts"); + expect(stderr).not.toContain("Cannot use 'export' in a CommonJS module"); + expect({ stdout, exitCode }).toEqual({ stdout: "ran\n", exitCode: 0 }); + }); + + test('export in .ts under "type":"commonjs" is not an error (TypeScript excluded)', async () => { + using dir = tempDir("tc-export-ts", { + "package.json": `{"type":"commonjs"}`, + "t.ts": exportBody, + }); + const { stdout, stderr, exitCode } = await run(String(dir), "t.ts"); + expect(stderr).not.toContain("Cannot use 'export' in a CommonJS module"); + expect({ stdout, exitCode }).toEqual({ stdout: "ran\n", exitCode: 0 }); + }); + + test("Bun.build does not reject export in .cjs (bundler has its own format resolution)", async () => { + using dir = tempDir("cjs-build", { "t.cjs": exportBody }); + const result = await Bun.build({ entrypoints: [join(String(dir), "t.cjs")], target: "bun" }); + expect(result.logs.filter(l => l.level === "error")).toEqual([]); + expect(result.success).toBe(true); + }); +}); diff --git a/test/cli/test/bun-test.test.ts b/test/cli/test/bun-test.test.ts index f720630d88c4..94b07f5bfb45 100644 --- a/test/cli/test/bun-test.test.ts +++ b/test/cli/test/bun-test.test.ts @@ -181,12 +181,15 @@ describe("bun test", () => { expect(stderr).toContain("test #1"); }); test("works with cjs dynamic import", () => { + // Top-level `await` is not valid in CommonJS; use `.then()` so the + // `.cjs` fixture actually is CommonJS. const cwd = createTest( ` - const { test, expect } = await import("bun:test"); - test("test #1", () => { - expect().pass(); - }) + import("bun:test").then(({ test, expect }) => { + test("test #1", () => { + expect().pass(); + }); + }); `, "test.test.cjs", ); diff --git a/test/js/bun/resolve/resolve-ts.test.ts b/test/js/bun/resolve/resolve-ts.test.ts index f0e121307bd3..d8d602e92ee3 100644 --- a/test/js/bun/resolve/resolve-ts.test.ts +++ b/test/js/bun/resolve/resolve-ts.test.ts @@ -42,9 +42,16 @@ function runTest( const files: Record = {}; if (jsFile) { - files[asDir ? "node_modules/abc/dir/index.js" : "node_modules/abc/index.js"] = - "export * from './sibling'; export const foo = 1;"; - files[asDir ? "node_modules/abc/dir/sibling.js" : "node_modules/abc/sibling.js"] = "export const sibling = 1;"; + // ESM `export` in a .js file under "type":"commonjs" is a SyntaxError in + // Node (and now in Bun), so write CJS-shaped sources for that case. The + // resolution behaviour under test is the same either way. + const isCjs = type === "commonjs"; + files[asDir ? "node_modules/abc/dir/index.js" : "node_modules/abc/index.js"] = isCjs + ? "Object.assign(exports, require('./sibling')); exports.foo = 1;" + : "export * from './sibling'; export const foo = 1;"; + files[asDir ? "node_modules/abc/dir/sibling.js" : "node_modules/abc/sibling.js"] = isCjs + ? "exports.sibling = 1;" + : "export const sibling = 1;"; } if (withPackageJSON) { diff --git a/test/js/bun/typescript/type-export.test.ts b/test/js/bun/typescript/type-export.test.ts index fc3b9538b1d4..0a77ea1de1a1 100644 --- a/test/js/bun/typescript/type-export.test.ts +++ b/test/js/bun/typescript/type-export.test.ts @@ -266,11 +266,10 @@ describe("through export merge", () => { test.concurrent(file, async () => { const result = await run([bunExe(), file], dir); - expect(result.stderr.trim()).toInclude( - file === "a." + fmt - ? 'error: Multiple exports with the same name "value"\n' // bun's syntax error - : "SyntaxError: Cannot export a duplicate name 'value'.\n", // jsc's syntax error - ); + // Bun's parser error surfaces whether `a` is the entrypoint or + // reached via import (the async transpile path now propagates + // parser log errors instead of handing the broken output to JSC). + expect(result.stderr.trim()).toInclude('error: Multiple exports with the same name "value"\n'); expect(result.exitCode).toBe(1); });