diff --git a/src/jsc/NodeCompileCache.rs b/src/jsc/NodeCompileCache.rs index 40d9b313a2e0..353dec9d3d5d 100644 --- a/src/jsc/NodeCompileCache.rs +++ b/src/jsc/NodeCompileCache.rs @@ -5,11 +5,12 @@ use core::sync::atomic::{AtomicBool, AtomicU8, Ordering}; use bstr::ByteSlice; +use bun_ast::Loader; use bun_boringssl::c as boring; use bun_collections::{HashMap, IdentityContext}; use bun_core::String as BunString; use bun_core::{Mutex, ZStr, env_var}; -use bun_options_types::Format; +use bun_options_types::{Format, ModuleType}; use bun_paths::{MAX_PATH_BYTES, PathBuffer, SEP}; use bun_sys::{self as sys, Fd, O}; @@ -511,6 +512,38 @@ pub fn get_dir() -> Option> { // Fetch-time hook (read + validate) // ────────────────────────────────────────────────────────────────────────── +/// Guarded [`fetch`] for the transpile paths (synchronous and concurrent): +/// only file-backed, JS-like modules participate. `code` is the exact +/// post-transpile byte text and is copied, so callers may reuse or replace +/// their buffer afterwards. Returns `(null, 0)` when the cache is disabled, +/// the module does not participate, or no on-disk entry validates. +pub fn fetch_for_transpiled_module( + path: &bun_paths::fs::Path<'_>, + loader: Loader, + is_cjs: bool, + code: &[u8], +) -> (*mut u8, usize) { + if !is_enabled() || !path.is_file() || !loader.is_java_script_like() { + return (core::ptr::null_mut(), 0); + } + fetch(path.text, is_cjs, code).unwrap_or((core::ptr::null_mut(), 0)) +} + +/// Guarded [`note_parse_failure`] for the transpile paths: register the +/// failed module so exit-time persist logs the "was not initialized" skip +/// (Node parity). `module_type` is the extension-sniffed type +/// ([`ModuleType::from_extension`]); `Unknown` records as CommonJS, like Node. +pub fn note_parse_failure_for_module( + path: &bun_paths::fs::Path<'_>, + loader: Loader, + module_type: ModuleType, +) { + if !is_enabled() || !path.is_file() || !loader.is_java_script_like() { + return; + } + note_parse_failure(path.text, !matches!(module_type, ModuleType::Esm)); +} + /// Module-fetch hook: register/refresh the entry for `filename`; returns the /// validated bytecode blob when the on-disk cache matches `code` (post- /// transpile text). The pointer stays valid for the process (entry map owns it). diff --git a/src/jsc/RuntimeTranspilerStore.rs b/src/jsc/RuntimeTranspilerStore.rs index 016b9bd524a2..a2b6f59cce20 100644 --- a/src/jsc/RuntimeTranspilerStore.rs +++ b/src/jsc/RuntimeTranspilerStore.rs @@ -892,6 +892,13 @@ impl TranspilerJob { } } + // Extension-sniffed, not the package.json-only `module_type` the + // parse used, so both transpile paths record the same type. + crate::node_compile_cache::note_parse_failure_for_module( + &path, + loader, + ModuleType::from_extension(path.name().ext, module_type), + ); self.parse_error = Some(crate::CrateError::ParseError); return; }; @@ -961,6 +968,21 @@ impl TranspilerJob { ptr::null_mut() }; + let is_commonjs_module = entry.metadata.module_type == CacheModuleType::Cjs; + // UTF-16 transpiler-cache output cannot byte-match the printed + // form, so it never reaches the compile cache. + let (bytecode_cache, bytecode_cache_size) = if matches!(&entry.output_code, OutputCode::String(s) if s.is_utf16()) + { + (ptr::null_mut(), 0) + } else { + crate::node_compile_cache::fetch_for_transpiled_module( + &path, + loader, + is_commonjs_module, + entry.output_code.byte_slice(), + ) + }; + self.resolved_source = OwnedResolvedSource::from(ResolvedSource { source_code: match &mut entry.output_code { OutputCode::String(s) => *s, @@ -970,9 +992,11 @@ impl TranspilerJob { result } }, - is_commonjs_module: entry.metadata.module_type == CacheModuleType::Cjs, + is_commonjs_module, module_info, tag: this_tag, + bytecode_cache, + bytecode_cache_size, ..Default::default() }); @@ -1132,6 +1156,14 @@ impl TranspilerJob { dump_source(vm, specifier, source_code_printer); } + let (bytecode_cache, bytecode_cache_size) = + crate::node_compile_cache::fetch_for_transpiled_module( + &path, + loader, + is_commonjs_module, + source_code_printer.ctx.get_written(), + ); + let source_code = 'brk: { let written = source_code_printer.ctx.get_written(); @@ -1172,6 +1204,8 @@ impl TranspilerJob { }) .unwrap_or(ptr::null_mut()), tag: this_tag, + bytecode_cache, + bytecode_cache_size, ..Default::default() }); diff --git a/src/options_types/bundle_enums.rs b/src/options_types/bundle_enums.rs index ba7eb1429491..7b99529fc315 100644 --- a/src/options_types/bundle_enums.rs +++ b/src/options_types/bundle_enums.rs @@ -112,6 +112,20 @@ pub enum ModuleType { impl ModuleType { pub const LIST: __ComptimeStringMap_MODULE_TYPE_LIST = __ComptimeStringMap_MODULE_TYPE_LIST(()); + + /// Module type from a file extension (dot included). The extension is + /// authoritative; `package_json_type` (the enclosing package.json + /// `"type"`, [`ModuleType::Unknown`] when absent) applies only to + /// `.js`/`.ts`, and other extensions (`.jsx`, `.tsx`, ...) stay + /// [`ModuleType::Unknown`] so the file contents decide. + pub fn from_extension(ext: &[u8], package_json_type: ModuleType) -> ModuleType { + match ext { + b".cjs" | b".cts" => ModuleType::Cjs, + b".mjs" | b".mts" => ModuleType::Esm, + b".js" | b".ts" => package_json_type, + _ => ModuleType::Unknown, + } + } } bun_core::comptime_string_map! { diff --git a/src/runtime/jsc_hooks.rs b/src/runtime/jsc_hooks.rs index 538ccf7e3715..79dbb1b87f0d 100644 --- a/src/runtime/jsc_hooks.rs +++ b/src/runtime/jsc_hooks.rs @@ -2025,21 +2025,6 @@ fn to_jsc_fetch_error(err: &crate::Error) -> bun_jsc::CrateError { _ => bun_jsc::CrateError::ParseError, } } -/// Shared guard for the two parse-failure exits: register the module with the -/// Node compile cache (Unknown module type maps to CJS, matching Node). -fn note_compile_cache_parse_failure( - path: &bun_resolver::fs::Path<'_>, - loader: Loader, - module_type: ModuleType, -) { - if bun_jsc::node_compile_cache::is_enabled() && loader.is_java_script_like() && path.is_file() { - bun_jsc::node_compile_cache::note_parse_failure( - path.text, - !matches!(module_type, ModuleType::Esm), - ); - } -} - /// `ModuleLoader.transpileSourceCode(...)` — the runtime-transpiler path: /// read file → `Transpiler::parse` /// → `js_printer::print` → `ResolvedSource`. @@ -2608,9 +2593,11 @@ fn transpile_source_code_inner( ); } arena_guard.2 = false; // give_back_arena = false - // Node compile cache: record the failed module so exit-time - // persist logs the "was not initialized" skip (Node parity). - note_compile_cache_parse_failure(path, loader, module_type); + bun_jsc::node_compile_cache::note_parse_failure_for_module( + path, + loader, + module_type, + ); return Err(crate::Error::ParseError); }; @@ -2664,9 +2651,11 @@ fn transpile_source_code_inner( // `transpiler.log` was swapped to non-null `args.log` above. if unsafe { (*(*jsc_vm).transpiler.log).errors > 0 } { arena_guard.2 = false; - // Node compile cache: record the failed module so exit-time - // persist logs the "was not initialized" skip (Node parity). - note_compile_cache_parse_failure(path, loader, module_type); + bun_jsc::node_compile_cache::note_parse_failure_for_module( + path, + loader, + module_type, + ); return Err(crate::Error::ParseError); } @@ -2855,21 +2844,18 @@ fn transpile_source_code_inner( core::ptr::null_mut() }; let is_commonjs_module = entry.metadata.module_type == CacheModuleType::Cjs; - // Node compile cache hook (transpiler-cache-hit path); must - // read `output_code` before it is consumed below. UTF-16 - // output would hash differently than the print path — skip. - let node_compile_cache_blob = if bun_jsc::node_compile_cache::is_enabled() - && source.path.is_file() - && loader.is_java_script_like() - && !matches!(&entry.output_code, OutputCode::String(s) if s.is_utf16()) + // UTF-16 transpiler-cache output cannot byte-match the + // printed form, so it never reaches the compile cache. + let (bytecode_cache, bytecode_cache_size) = if matches!(&entry.output_code, OutputCode::String(s) if s.is_utf16()) { - bun_jsc::node_compile_cache::fetch( - source.path.text, + (core::ptr::null_mut(), 0) + } else { + bun_jsc::node_compile_cache::fetch_for_transpiled_module( + &source.path, + loader, is_commonjs_module, entry.output_code.byte_slice(), ) - } else { - None }; let source_code = match &mut entry.output_code { OutputCode::String(s) => *s, @@ -2929,8 +2915,6 @@ fn transpile_source_code_inner( } else { ResolvedSourceTag::Javascript }; - let (bytecode_cache, bytecode_cache_size) = - node_compile_cache_blob.unwrap_or((core::ptr::null_mut(), 0)); return Ok(OwnedResolvedSource::from(ResolvedSource { source_code, specifier: input_specifier.dupe_ref(), @@ -3121,14 +3105,13 @@ fn transpile_source_code_inner( let printer: &mut bun_js_printer::BufferPrinter = unsafe { &mut *(*extra).source_code_printer }; let written = printer.ctx.get_written(); - let node_compile_cache_blob = if bun_jsc::node_compile_cache::is_enabled() - && path.is_file() - && loader.is_java_script_like() - { - bun_jsc::node_compile_cache::fetch(path.text, is_commonjs_module, written) - } else { - None - }; + let (bytecode_cache, bytecode_cache_size) = + bun_jsc::node_compile_cache::fetch_for_transpiled_module( + path, + loader, + is_commonjs_module, + written, + ); // SAFETY: per fn contract — `jsc_vm` is the live per-thread // VM; `printer.ctx.get_written()` borrows thread-local data. let mut resolved_source = unsafe { @@ -3141,10 +3124,8 @@ fn transpile_source_code_inner( }; resolved_source.is_commonjs_module = is_commonjs_module; resolved_source.module_info = module_info; - if let Some((ptr, size)) = node_compile_cache_blob { - resolved_source.bytecode_cache = ptr; - resolved_source.bytecode_cache_size = size; - } + resolved_source.bytecode_cache = bytecode_cache; + resolved_source.bytecode_cache_size = bytecode_cache_size; return Ok(OwnedResolvedSource::from(resolved_source)); } @@ -3205,16 +3186,13 @@ fn transpile_source_code_inner( let printer: &mut bun_js_printer::BufferPrinter = unsafe { &mut *(*extra).source_code_printer }; let written = printer.ctx.get_written(); - // Node compile cache hook (sync transpile path). `fetch` copies - // `written`; the printer may be replaced below. - let node_compile_cache_blob = if bun_jsc::node_compile_cache::is_enabled() - && path.is_file() - && loader.is_java_script_like() - { - bun_jsc::node_compile_cache::fetch(path.text, is_commonjs_module, written) - } else { - None - }; + let (bytecode_cache, bytecode_cache_size) = + bun_jsc::node_compile_cache::fetch_for_transpiled_module( + path, + loader, + is_commonjs_module, + written, + ); // The `Jsc` vtable bridge `put()` does not write // `cache.output_code` (only the `r#impl == None` fallback // does, and `r#impl` is `Some(Jsc)` here), so it is always @@ -3236,8 +3214,6 @@ fn transpile_source_code_inner( // (fd close handled by `_fd_guard` registered above; spec // :251-256 `defer` fires on every exit path.) - let (bytecode_cache, bytecode_cache_size) = - node_compile_cache_blob.unwrap_or((core::ptr::null_mut(), 0)); return Ok(OwnedResolvedSource::from(ResolvedSource { source_code, specifier: input_specifier.dupe_ref(), @@ -4344,34 +4320,12 @@ unsafe fn transpile_file( } // ── module_type sniff from extension / package.json ───────────────────── - let module_type: ModuleType = 'brk: { - let ext = lr.path.name().ext; - // regex /\.[cm][jt]s$/ - if ext.len() == b".cjs".len() { - if ext == b".cjs" { - break 'brk ModuleType::Cjs; - } - if ext == b".mjs" { - break 'brk ModuleType::Esm; - } - if ext == b".cts" { - break 'brk ModuleType::Cjs; - } - if ext == b".mts" { - break 'brk ModuleType::Esm; - } - } - // 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); - } - // For JSX/TSX and other extensions, let the file contents decide. - ModuleType::Unknown - }; + let module_type = ModuleType::from_extension( + lr.path.name().ext, + lr.package_json + .map(|pkg| pkg.module_type) + .unwrap_or(ModuleType::Unknown), + ); let pkg_name: Option<&[u8]> = lr .package_json .and_then(|pkg| (!pkg.name.is_empty()).then_some(&*pkg.name)); @@ -4401,9 +4355,6 @@ unsafe fn transpile_file( // TODO: allow running concurrently when no onLoad handlers match a plugin. && plugin_runner_is_none && store_enabled - // With the Node compile cache enabled, transpile on-thread so the - // fetch hook sees every module. - && !bun_jsc::node_compile_cache::is_enabled() { // Disgusting workaround: polyfills like // `reflect-metadata` are CJS-with-side-effects that other ESM diff --git a/test/js/node/module/node-module-module.test.js b/test/js/node/module/node-module-module.test.js index d0e722444fbd..374c29a6fd3f 100644 --- a/test/js/node/module/node-module-module.test.js +++ b/test/js/node/module/node-module-module.test.js @@ -111,6 +111,62 @@ describe.concurrent("node-module-module", () => { expect(exitCode).toBe(0); }); + test("compile cache does not change ESM evaluation order", async () => { + // A `.cjs` sibling with no CommonJS features is loaded as ESM and must + // evaluate after the `.mjs` module imported before it, with or without + // the compile cache (cold and warm), and via the API enable path. + using dir = tempDir("compile-cache-eval-order", { + "main.mjs": `import "./e.mjs";\nimport "./c.cjs";\nconsole.log(globalThis.o.join(","));`, + "e.mjs": `(globalThis.o ??= []).push("esm");\nexport {};`, + "c.cjs": `(globalThis.o ??= []).push("cjs");`, + "preload.cjs": `require("node:module").enableCompileCache(__dirname + "/cc-api");`, + }); + const cacheDir = path.join(String(dir), "cc"); + const apiCacheDir = path.join(String(dir), "cc-api"); + const run = async (args, env) => { + await using proc = Bun.spawn({ + cmd: [bunExe(), ...args], + env: { ...bunEnv, NODE_COMPILE_CACHE: undefined, ...env }, + cwd: String(dir), + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stdout.trim()).toBe("esm,cjs"); + expect(stderr).toBe(""); + expect(exitCode).toBe(0); + }; + await run(["main.mjs"], {}); + await run(["main.mjs"], { NODE_COMPILE_CACHE: cacheDir }); // cold + await run(["main.mjs"], { NODE_COMPILE_CACHE: cacheDir }); // warm + await run(["--preload", "./preload.cjs", "main.mjs"], {}); + // The cached runs only prove anything if the cache was actually active. + for (const dirToCheck of [cacheDir, apiCacheDir]) { + const tagged = fs.readdirSync(dirToCheck); + expect(tagged).toHaveLength(1); + expect(fs.readdirSync(path.join(dirToCheck, tagged[0])).length).toBeGreaterThanOrEqual(3); + } + }); + + test("compile cache records a parse-failed .mjs as ESM regardless of package type", async () => { + // Parse-failure bookkeeping keys off the file extension like the + // synchronous loader and Node, not the enclosing package.json "type". + using dir = tempDir("compile-cache-parse-failure-type", { + "main.mjs": `import "./pkg/broken.mjs";`, + "pkg/package.json": `{"type":"commonjs"}`, + "pkg/broken.mjs": `import {;`, + }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "main.mjs"], + env: { ...bunEnv, NODE_COMPILE_CACHE: path.join(String(dir), "cc"), NODE_DEBUG_NATIVE: "COMPILE_CACHE" }, + cwd: String(dir), + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stdout).toBe(""); + expect(stderr).toMatch(/skip persisting ESM file:.*broken\.mjs because the cache was not initialized/); + expect(exitCode).not.toBe(0); + }); + test.skipIf(process.platform === "win32")( "compile cache persists modules loaded after a non-fatal self-kill", async () => {