diff --git a/crates/nub-cache-key/src/lib.rs b/crates/nub-cache-key/src/lib.rs index 6501b77cf..65e0340ce 100644 --- a/crates/nub-cache-key/src/lib.rs +++ b/crates/nub-cache-key/src/lib.rs @@ -1,9 +1,10 @@ -//! Transpile-cache key derivation, factored out of `nub-native` so the -//! invalidation contract is unit-testable (see this crate's Cargo.toml for why a -//! test can't live in the cdylib `nub-native`). +//! The transpile cache's pure core, factored out of `nub-native` so it is +//! unit-testable (see this crate's Cargo.toml for why a test can't live in the +//! cdylib `nub-native`): the key derivation here, and in [`source_url`] the +//! `//# sourceURL` spelling the key has to cover. //! //! The key preimage is NUL-separated and order-fixed (no trailing separator): -//! `nub_version \0 schema \0 build_id \0 source \0 ext \0 tsconfig_hash \0 pkg_type \0 filename` +//! `nub_version \0 schema \0 build_id \0 source \0 ext \0 tsconfig_hash \0 pkg_type \0 filename \0 url_band` //! → blake3 → 64-hex lowercase → the cache FILENAME. Every component is hashed //! IN, so a change to ANY of them (a nub release, a CACHE_SCHEMA bump, a rebuild //! at a new build-id) yields a disjoint filename — old on-disk entries are @@ -11,10 +12,18 @@ //! its `NUB_VERSION` / `CACHE_SCHEMA` / `BUILD_ID` consts. //! //! `filename` is in the preimage because the cached body carries a per-file -//! `//# sourceURL=` magic comment: two byte-identical sources in +//! `//# sourceURL=` magic comment: two byte-identical sources in //! the same directory would otherwise collide on one entry and the second file //! would be served the first's `sourceURL`, misattributing V8 stack frames and //! debugger source mapping (issue #171). +//! +//! `url_band` is in the preimage for the same reason one step further out: that +//! sourceURL is spelled to match the HOST Node's `pathToFileURL`, whose escape set +//! changed mid-release-line ([`source_url::host_widens_url_path`]). Without it, a +//! project built under one Node and then run under another — an `nvm use`, or +//! `nub node pin` — would be served a body carrying the other band's spelling. + +pub mod source_url; /// blake3 of the NUL-separated key preimage → 64-hex lowercase. /// @@ -22,7 +31,9 @@ /// components (compile-time consts on the production path); `source` / `ext` / /// `tsconfig_hash` / `pkg_type` / `filename` are the per-file inputs. `filename` /// is keyed in because it is baked into the cached body as the `//# sourceURL` -/// comment, so distinct files must not share an entry (issue #171). +/// comment, so distinct files must not share an entry (issue #171); `url_band` +/// ([`source_url::band_tag`]) is keyed in because that same comment is spelled to +/// match the host Node, so one file has two valid bodies across hosts. #[allow(clippy::too_many_arguments)] pub fn cache_key( nub_version: &str, @@ -33,6 +44,7 @@ pub fn cache_key( tsconfig_hash: &str, pkg_type: &str, filename: &str, + url_band: &str, ) -> String { let mut h = blake3::Hasher::new(); h.update(nub_version.as_bytes()); @@ -50,6 +62,8 @@ pub fn cache_key( h.update(pkg_type.as_bytes()); h.update(b"\0"); h.update(filename.as_bytes()); + h.update(b"\0"); + h.update(url_band.as_bytes()); h.finalize().to_hex().to_string() } @@ -63,9 +77,12 @@ mod tests { const TSCONFIG: &str = "tsconfig-hash"; const PKG: &str = "module"; const FILENAME: &str = "/proj/src/x.ts"; + const BAND: &str = "url-wide"; fn key(version: &str, schema: &str, build_id: &str) -> String { - cache_key(version, schema, build_id, SRC, EXT, TSCONFIG, PKG, FILENAME) + cache_key( + version, schema, build_id, SRC, EXT, TSCONFIG, PKG, FILENAME, BAND, + ) } /// A rebuilt binary (new build-id) must not serve a prior build's entries: @@ -115,6 +132,7 @@ mod tests { TSCONFIG, PKG, "/proj/alpha.ts", + BAND, ); let beta = cache_key( "0.0.1", @@ -125,10 +143,36 @@ mod tests { TSCONFIG, PKG, "/proj/beta.ts", + BAND, ); assert_ne!( alpha, beta, "identical content at different paths must yield distinct cache keys" ); } + + /// The cached body carries a sourceURL spelled for the host Node's escape + /// band, so the same file under two hosts must not share one entry — this is + /// what stops `nvm use` from serving the other band's spelling. + #[test] + fn cache_key_distinguishes_the_url_bands() { + let wide = cache_key( + "0.0.1", "6", "abc1234", SRC, EXT, TSCONFIG, PKG, FILENAME, "url-wide", + ); + let narrow = cache_key( + "0.0.1", + "6", + "abc1234", + SRC, + EXT, + TSCONFIG, + PKG, + FILENAME, + "url-narrow", + ); + assert_ne!( + wide, narrow, + "the two sourceURL escape bands must yield distinct cache keys" + ); + } } diff --git a/crates/nub-cache-key/src/source_url.rs b/crates/nub-cache-key/src/source_url.rs new file mode 100644 index 000000000..ec7dae739 --- /dev/null +++ b/crates/nub-cache-key/src/source_url.rs @@ -0,0 +1,287 @@ +//! The `//# sourceURL` a transpiled body carries, spelled exactly as the HOST +//! Node's `pathToFileURL(path).href` would spell it. +//! +//! Node's loader computes a module's own URL with `pathToFileURL`, so a sourceURL +//! that disagrees with it gives one file two identities inside a single process: +//! `Debugger.scriptParsed` reports nub's spelling while a breakpoint set against +//! the module URL carries Node's, and the two never match. That is why the +//! encoding is pinned to the host rather than to the URL spec. +//! +//! The URL FORM is not host-gated. Node's own type stripping only started +//! reporting a `file:` URL in v26.4.0 (backported to v24.19.0); before that it +//! reported the bare path, and v25.x never got the change. Reproducing that would +//! flip nub's script identity on a patch bump, so nub always emits the URL — the +//! form upstream is migrating toward. Only the ENCODING, which decides whether +//! two URLs for one file are equal, follows the host. +//! +//! The unconditional form is not merely forward-looking. On a v22.15 or v24.10 +//! host an ordinary `.js` CommonJS module ALREADY reports a `file:` URL, because +//! nub does not intercept no-op plain JS and Node names it itself. The bare path +//! therefore made a `.ts` file the odd one out among its own siblings in one +//! process, not just different from a future Node. + +/// Whether the host's `pathToFileURL` percent-encodes the widened characters +/// `[`, `]`, `^`, `|` and `~`. +/// +/// nodejs/node#54545 widened the escape set, and it reached the release lines out +/// of order, so this cannot be reduced to a single floor. All five characters move +/// together in one step — verified by sweeping every printable byte through real +/// binaries at each boundary tag: +/// +/// | Host band | Widened | +/// | --- | --- | +/// | 18.x (through 18.20.8) | no | +/// | 20.0.0 – 20.18.2 | no | +/// | 20.18.3+ | yes | +/// | 21.x (through 21.7.3) | no | +/// | 22.0.0 – 22.11.x | no | +/// | 22.12.0+ | yes | +/// | 23.0.x | no | +/// | 23.1.0+ | yes | +/// | 24.0.0+ | yes | +/// +/// v20.18.3 and v22.12.0 are the same behavior by two different mechanisms — +/// v20/v22 got widened regexes in the pure-JS `pathToFileURL`, and the move to +/// ada's native `href_from_file` came later — so the band is about the escape +/// set, not about which implementation produces it. +/// +/// An unparseable or absent version reads as WIDENED: every release line still +/// receiving patches is above its boundary, so that is the answer that is right +/// for anything nub will actually meet. +pub fn host_widens_url_path(node_version: &str) -> bool { + let Some((major, minor, patch)) = parse_version(node_version) else { + return true; + }; + match major { + 0..=19 => false, + 20 => (minor, patch) >= (18, 3), + 21 => false, + 22 => minor >= 12, + 23 => minor >= 1, + _ => true, + } +} + +/// `v22.12.0` / `22.12.0` → `(22, 12, 0)`. Trailing prerelease or build metadata +/// is ignored; anything that does not start with three numeric components is +/// rejected so the caller can apply its own default. +fn parse_version(raw: &str) -> Option<(u32, u32, u32)> { + let trimmed = raw.strip_prefix('v').unwrap_or(raw); + let mut parts = trimmed.split('.'); + let major = parts.next()?.parse().ok()?; + let minor = parts.next()?.parse().ok()?; + let tail = parts.next()?; + let end = tail + .find(|c: char| !c.is_ascii_digit()) + .unwrap_or(tail.len()); + let patch = tail[..end].parse().ok()?; + Some((major, minor, patch)) +} + +/// The cache-key component naming the encoding band, so a body emitted for one +/// band is never served to a host on the other. +pub fn band_tag(widened: bool) -> &'static str { + if widened { "url-wide" } else { "url-narrow" } +} + +/// Whether a byte must be percent-encoded inside a `file:` URL path. +/// +/// The always-escaped set is Node's `encodePathChars` plus the WHATWG path +/// percent-encode set, which between them cover the C0 controls, space, `"`, `#`, +/// `%`, `<`, `>`, `?`, `\`, backtick, `{`, `}`, DEL and every non-ASCII byte. The +/// `widened` five ride on [`host_widens_url_path`]. Both halves were read off real +/// binaries rather than off the URL spec, because the spec set matches neither +/// band exactly. +fn must_percent_encode(b: u8, widened: bool) -> bool { + if !b.is_ascii() { + return true; + } + if widened && matches!(b, b'[' | b']' | b'^' | b'|' | b'~') { + return true; + } + matches!( + b, + 0x00..=0x20 | b'"' | b'#' | b'%' | b'<' | b'>' | b'?' | b'\\' | b'`' | b'{' | b'}' | 0x7f + ) +} + +fn push_encoded(out: &mut String, s: &str, widened: bool) { + const HEX: &[u8; 16] = b"0123456789ABCDEF"; + for b in s.bytes() { + if must_percent_encode(b, widened) { + out.push('%'); + out.push(HEX[(b >> 4) as usize] as char); + out.push(HEX[(b & 0x0f) as usize] as char); + } else { + out.push(b as char); + } + } +} + +/// The `file:` URL spelling of `path`, byte-identical to the host's +/// `pathToFileURL(path).href`. +/// +/// `path` always arrives absolute in production (the loader derives it with +/// `fileURLToPath`); anything else is passed through unconverted, matching Node's +/// `convertCJSFilenameToURL` fallback. `windows` is a parameter rather than a +/// `cfg!` so both branches are exercisable from either host. +pub fn file_url(path: &str, windows: bool, widened: bool) -> String { + let mut url = String::with_capacity(path.len() + 16); + if !windows { + if !path.starts_with('/') { + return path.to_string(); + } + url.push_str("file://"); + push_encoded(&mut url, path, widened); + return url; + } + let forward = path.replace('\\', "/"); + if let Some(rest) = forward.strip_prefix("//") { + // UNC `\\server\share\…` → `file://server/share/…`. The hostname is a URL + // authority rather than a path segment, so it is not percent-encoded. + let host_end = rest.find('/').unwrap_or(rest.len()); + url.push_str("file://"); + url.push_str(&rest[..host_end]); + push_encoded(&mut url, &rest[host_end..], widened); + return url; + } + let bytes = forward.as_bytes(); + let drive_absolute = + bytes.len() >= 3 && bytes[0].is_ascii_alphabetic() && bytes[1] == b':' && bytes[2] == b'/'; + if !drive_absolute { + return path.to_string(); + } + url.push_str("file:///"); + push_encoded(&mut url, &forward, widened); + url +} + +#[cfg(test)] +mod tests { + use super::{file_url, host_widens_url_path}; + + /// The band boundaries, taken from running `pathToFileURL('/xy')` over + /// every printable ASCII byte on real binaries at each tag — including the + /// three that pin the v20 boundary (20.18.2 narrow, 20.18.3 wide) and the last + /// release of each line that never widened (18.20.8, 21.7.3). + #[test] + fn band_boundaries_match_the_measured_releases() { + for narrow in [ + "v18.19.0", "v18.20.8", "v20.10.0", "v20.18.0", "v20.18.1", "v20.18.2", "v21.1.0", + "v21.7.3", "v22.6.0", "v22.11.0", "v23.0.0", + ] { + assert!( + !host_widens_url_path(narrow), + "{narrow} does not escape [ ] ^ | ~" + ); + } + for wide in [ + "v20.18.3", "v20.19.0", "v22.12.0", "v22.15.0", "v23.1.0", "v23.3.0", "v24.0.0", + "v26.7.0", + ] { + assert!(host_widens_url_path(wide), "{wide} escapes [ ] ^ | ~"); + } + } + + /// A version nub cannot parse must not silently pick the legacy spelling: a + /// nightly, a `-pre` build, or a future line is far likelier to be widened. + #[test] + fn unparseable_version_defaults_to_widened() { + for odd in ["", "not-a-version", "v99", "v27.0.0-pre"] { + assert!(host_widens_url_path(odd), "{odd:?} must read as widened"); + } + } + + /// The characters that move with the band, and the ones that must not. + #[test] + fn widened_characters_follow_the_band() { + assert_eq!( + file_url("/w[x]y^z~q|r", false, true), + "file:///w%5Bx%5Dy%5Ez%7Eq%7Cr" + ); + assert_eq!( + file_url("/w[x]y^z~q|r", false, false), + "file:///w[x]y^z~q|r" + ); + // Escaped in BOTH bands — captured from v18.19.0 and v26.7.0 alike. + for band in [true, false] { + assert_eq!( + file_url("/a b%c#d?e", false, band), + "file:///a%20b%25c%23d%3Fe" + ); + // On POSIX a backslash is an ordinary filename byte, never a separator. + assert_eq!(file_url("/x\\y", false, band), "file:///x%5Cy"); + } + } + + /// Non-ASCII goes out as percent-encoded UTF-8, uppercase hex, in both bands. + #[test] + fn non_ascii_is_percent_encoded_utf8() { + for band in [true, false] { + assert_eq!(file_url("/xé€y", false, band), "file:///x%C3%A9%E2%82%ACy"); + } + } + + /// Windows drive paths, against `pathToFileURL(p, { windows: true }).href` + /// captured on v26.7.0: backslashes become separators (never `%5C`, unlike + /// POSIX), and the drive letter sits in the path, not the authority. + #[test] + fn windows_drive_paths_match_captured_output() { + assert_eq!( + file_url(r"C:\a b\x~y.ts", true, true), + "file:///C:/a%20b/x%7Ey.ts" + ); + assert_eq!( + file_url(r"C:\a b\x~y.ts", true, false), + "file:///C:/a%20b/x~y.ts" + ); + assert_eq!( + file_url(r"C:\a%b\x.ts", true, true), + "file:///C:/a%25b/x.ts" + ); + assert_eq!( + file_url(r"C:\a\x#y?z.ts", true, true), + "file:///C:/a/x%23y%3Fz.ts" + ); + assert_eq!( + file_url(r"C:\a\x[y]^z|w.ts", true, true), + "file:///C:/a/x%5By%5D%5Ez%7Cw.ts" + ); + assert_eq!( + file_url(r"C:\a\x[y]^z|w.ts", true, false), + "file:///C:/a/x[y]^z|w.ts" + ); + // An already-forward-slashed drive path is accepted unchanged. + assert_eq!(file_url("C:/a/b.ts", true, true), "file:///C:/a/b.ts"); + } + + /// UNC paths, same capture: the share host is the URL authority and stays + /// unencoded, while the path after it is encoded normally. + #[test] + fn windows_unc_paths_keep_the_host_unencoded() { + assert_eq!( + file_url(r"\\server\share\a b\x.ts", true, true), + "file://server/share/a%20b/x.ts" + ); + assert_eq!( + file_url(r"\\server\share\w[x]~y.ts", true, true), + "file://server/share/w%5Bx%5D%7Ey.ts" + ); + assert_eq!( + file_url(r"\\server\share\w[x]~y.ts", true, false), + "file://server/share/w[x]~y.ts" + ); + } + + /// A non-absolute input is passed through untouched on either platform, + /// matching Node's `convertCJSFilenameToURL`, which only converts when + /// `isAbsolute(filename)`. + #[test] + fn non_absolute_input_is_left_alone() { + assert_eq!(file_url("[eval]", false, true), "[eval]"); + assert_eq!(file_url("relative/x.ts", false, true), "relative/x.ts"); + assert_eq!(file_url(r"relative\x.ts", true, true), r"relative\x.ts"); + // Rooted but not drive-qualified is not absolute on Windows. + assert_eq!(file_url("/rooted/x.ts", true, true), "/rooted/x.ts"); + } +} diff --git a/crates/nub-cli/tests/ts_source_url.rs b/crates/nub-cli/tests/ts_source_url.rs new file mode 100644 index 000000000..945d75de8 --- /dev/null +++ b/crates/nub-cli/tests/ts_source_url.rs @@ -0,0 +1,158 @@ +//! A transpiled TypeScript file's script identity is the `file:` URL Node's own +//! type-stripping reports, not a bare filesystem path. +//! +//! nub appends a `//# sourceURL` to every transpiled body, and that value is what +//! the inspector reports as `Debugger.scriptParsed`'s `url` — the key editors and +//! DevTools match breakpoints against. Node reports the module's URL there +//! (`pathToFileURL(filename).href`, via `convertCJSFilenameToURL`); emitting the +//! path instead gave every `.ts` file a different identity under nub than under +//! `node`. Upstream pins the same contract in +//! `test/parallel/test-inspector-strip-types.js`, including `hasSourceURL`. +//! +//! Deliberately NOT asserted here: the rendered stack frame. nub runs with +//! `--enable-source-maps`, and Node's source-map frame formatter strips the +//! `file://` scheme off the mapped source by design +//! (`prepare_stack_trace.js`, `originalSourceNoScheme`), so a mapped frame shows a +//! bare path whatever the sourceURL says. The inspector URL is the surface this +//! bug is actually about. +//! +//! The fixture supplies its OWN expectation — `pathToFileURL(__filename).href` +//! plus URLs resolved against it — so the assertion is a differential against +//! Node's URL for the same files rather than path arithmetic repeated in the +//! test. Each fixture directory is run TWICE against a private cache directory: +//! the sourceURL is baked into the cached body, so a cold transform and a warm +//! cache hit are separate claims. + +use std::path::{Path, PathBuf}; +use std::process::Command; + +fn nub_binary() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_nub")) +} + +fn host_node_usable() -> bool { + Command::new("node") + .arg("--version") + .output() + .is_ok_and(|out| out.status.success()) +} + +/// A `.ts` entry that pulls in one dependency per module format, so all three +/// load paths (entry, `require` of `.cts`, `import` of `.mts`) are covered by one +/// process. `Debugger.enable` is posted before either dependency loads, which is +/// what makes their `scriptParsed` notifications observable. +const PROBE: &str = r#"const inspector = require("node:inspector"); +const { pathToFileURL } = require("node:url"); + +const session = new inspector.Session(); +session.connect(); +const seen: Record = {}; +session.on("Debugger.scriptParsed", ({ params }: any) => { + if (/\.[cm]?ts$/.test(params.url)) seen[params.url] = params.hasSourceURL; +}); +session.post("Debugger.enable", async () => { + require("./dep.cts"); + await import("./dep.mts"); + const self: string = pathToFileURL(__filename).href; + const expected: string[] = [self, new URL("dep.cts", self).href, new URL("dep.mts", self).href]; + console.log(JSON.stringify({ expected, seen })); +}); +"#; + +/// Write the fixture into `dir`, run it, and assert every TypeScript file in the +/// graph reported Node's own URL for itself as its script identity. +fn assert_script_urls(dir: &Path, cache: &Path) { + std::fs::create_dir_all(dir).unwrap(); + std::fs::write(dir.join("package.json"), r#"{ "type": "commonjs" }"#).unwrap(); + std::fs::write(dir.join("probe.ts"), PROBE).unwrap(); + std::fs::write( + dir.join("dep.cts"), + "const y: number = 2;\nmodule.exports = y;\n", + ) + .unwrap(); + std::fs::write(dir.join("dep.mts"), "export const z: number = 3;\n").unwrap(); + + for pass in ["cold", "warm"] { + let output = Command::new(nub_binary()) + .arg("probe.ts") + .current_dir(dir) + .env("XDG_CACHE_HOME", cache) + .output() + .expect("failed to spawn nub"); + assert!( + output.status.success(), + "nub exited {:?} ({pass}) in {}\nstderr: {}", + output.status, + dir.display(), + String::from_utf8_lossy(&output.stderr) + ); + let stdout = String::from_utf8_lossy(&output.stdout); + let parsed: serde_json::Value = serde_json::from_str(stdout.trim()) + .unwrap_or_else(|e| panic!("fixture must emit JSON ({pass}), got {stdout:?}: {e}")); + + for expected in parsed["expected"].as_array().unwrap() { + let href = expected.as_str().unwrap(); + assert!( + href.starts_with("file:///"), + "({pass}) the fixture's own oracle must be a file: URL, got {href:?}" + ); + assert_eq!( + parsed["seen"][href], + serde_json::json!(true), + "({pass}) {href} must reach the inspector under Node's own URL for it, \ + with hasSourceURL set.\n reported: {}", + parsed["seen"] + ); + } + + // Without this, a cache directory that never got written — the mkdirSync + // in transform-core.mjs swallows its error and disables the cache — would + // silently turn the warm pass into a second cold one, and the entry whose + // sourceURL is baked in would go untested. + if pass == "cold" { + let entries = std::fs::read_dir(cache.join("nub/transpile")) + .map(|d| d.count()) + .unwrap_or(0); + assert!( + entries > 0, + "the cold pass must leave entries in {}, or the warm pass tests nothing", + cache.join("nub/transpile").display() + ); + } + } +} + +/// The baseline contract, plus the host-band plumbing. +/// +/// The directory name carries four of the five characters whose escaping depends +/// on the host's Node version, so this is the end-to-end witness that +/// `process.version` reaches the encoder: hand `transformCached` the wrong value +/// and nub spells them the other way while the fixture's oracle — the host's own +/// `pathToFileURL` — does not, and the assertion fails. `|` is left out because +/// Windows forbids it in a filename; `[`, `]`, `^` and `~` are all legal there, so +/// the Windows leg exercises the same plumbing. +/// +/// The surrounding temp path has nothing to escape, so it covers the ordinary +/// no-op case in the same run. +#[test] +fn transpiled_typescript_reports_its_file_url_to_the_inspector() { + if !host_node_usable() { + return; + } + let temp = tempfile::tempdir().unwrap(); + assert_script_urls(&temp.path().join("w[x]y^z~q"), &temp.path().join("cache")); +} + +/// The characters Windows cannot put in a filename, so the test above can never +/// reach them: `%`, `#` and `?` from the always-escaped set, and `|`, the fifth +/// band-sensitive character. A naive `format!("file://{path}")` passes the test +/// above and fails here. +#[cfg(unix)] +#[test] +fn percent_encoded_path_matches_node() { + if !host_node_usable() { + return; + } + let temp = tempfile::tempdir().unwrap(); + assert_script_urls(&temp.path().join("a b%c#d?e|f"), &temp.path().join("cache")); +} diff --git a/crates/nub-native/src/cache.rs b/crates/nub-native/src/cache.rs index 68b18a8b1..aab66eebb 100644 --- a/crates/nub-native/src/cache.rs +++ b/crates/nub-native/src/cache.rs @@ -3,12 +3,14 @@ //! the old JS cache, so warm caches survive the JS→Rust move (no global miss). //! //! Cache key preimage (no trailing separator): -//! `NUB_VERSION \0 CACHE_SCHEMA \0 build_id \0 source \0 ext \0 tsconfig_hash \0 (pkg_type||"") \0 filename` -//! → blake3 → 64-hex lowercase → cache FILENAME. `filename` is in the key -//! because the cached body carries a per-file `//# sourceURL` comment (issue -//! #171). (SCHEMA "6" added `filename`; SCHEMA "5" = compile-time build-id era; -//! SCHEMA "4" folded a runtime exe-hash here, SCHEMA "3" / the old JS path used -//! SHA-256 and a key without that component.) +//! `NUB_VERSION \0 CACHE_SCHEMA \0 build_id \0 source \0 ext \0 tsconfig_hash \0 (pkg_type||"") \0 filename \0 url_band` +//! → blake3 → 64-hex lowercase → cache FILENAME. `filename` and `url_band` are +//! in the key because the cached body carries a per-file `//# sourceURL` +//! comment (issue #171) spelled for the host Node's escape band. (SCHEMA "7" = +//! the `//# sourceURL` file-URL switch, which also added `url_band`; SCHEMA "6" +//! added `filename`; SCHEMA "5" = compile-time build-id era; SCHEMA "4" folded a +//! runtime exe-hash here, SCHEMA "3" / the old JS path used SHA-256 and a key +//! without that component.) //! On-disk entry: `[16-hex integrity = blake3(body)[..16]][body]`, where //! `body = format_byte('c'|'m') + post_processed_code`. //! Atomic write via a `*.tmp` sibling + rename (the `*.tmp` suffix is what @@ -17,17 +19,23 @@ //! The post-processing that the old JS did in `loadTranspile` after `transform` //! moves in here so the cached bytes are the FINAL bytes: drop oxc's empty //! `export {};` marker for CJS, append the inline base64 sourceMap, append the -//! `//# sourceURL=` magic comment. +//! `//# sourceURL=` magic comment. use std::sync::atomic::{AtomicU64, Ordering}; use napi::Result; use napi_derive::napi; +use nub_cache_key::source_url; use oxc_napi::OxcError; use crate::transform::{TransformOptions, transform}; -/// On-disk entry format version. Bumped to "6" when `filename` was folded into +/// On-disk entry format version. Bumped to "7" when the `//# sourceURL` switched +/// from a bare filesystem path to the `file:` URL Node reports (see +/// [`source_url::file_url`]) — the sourceURL is baked into the stored body, so a +/// "6"-era entry would keep serving the old script identity. That same change +/// added `url_band` to the preimage, which is what keeps two hosts' spellings +/// apart; the schema stays at "7" because both landed together. "6" folded `filename` into /// the key (issue #171): a "5"-era entry was named without `filename` in the /// preimage, so two byte-identical sources in a directory collided on one entry /// and the second was served the first's `//# sourceURL`. "5" was the move from a @@ -35,7 +43,7 @@ use crate::transform::{TransformOptions, transform}; /// constant is hashed INTO the key, so the filenames are disjoint across schemas /// — old entries are silently ignored (a miss), never mis-read. (SCHEMA "4" was /// the blake3 + exe-hash era; "3" / the old JS path used SHA-256.) -const CACHE_SCHEMA: &str = "6"; +const CACHE_SCHEMA: &str = "7"; const INTEGRITY_LEN: usize = 16; /// Lockstep with `runtime/version.mjs` via `make version`; the sole version /// component of the key (a new nub release ships any emit change + a rebuilt addon). @@ -69,6 +77,11 @@ pub struct CachedTransformResult { /// the cached body; `cache_dir` is the JS enable/disable signal (`None` ⇒ skip all /// I/O, just transform). The remaining key components are JS-supplied so the key /// stays byte-identical to the old pipeline. +/// +/// `node_version` is `process.version` — the host whose `pathToFileURL` spelling +/// the emitted `//# sourceURL` has to match. It is passed rather than read here +/// because every other key component is already JS-supplied, and because it is an +/// INPUT to the cached bytes: the derived band goes into the key alongside them. #[allow(clippy::too_many_arguments, clippy::needless_pass_by_value)] #[napi] pub fn transform_cached( @@ -80,6 +93,7 @@ pub fn transform_cached( pkg_type: String, format_byte: String, cache_dir: Option, + node_version: String, ) -> Result { let format = if format_byte == "c" { "commonjs" @@ -87,7 +101,17 @@ pub fn transform_cached( "module" }; - let key = cache_key(&source, &ext, &tsconfig_hash, &pkg_type, &filename); + // The band, not the raw version: a patch bump inside one band emits identical + // bytes, so keying the version itself would drop a warm cache for nothing. + let widened = source_url::host_widens_url_path(&node_version); + let key = cache_key( + &source, + &ext, + &tsconfig_hash, + &pkg_type, + &filename, + source_url::band_tag(widened), + ); // Cache hit path. if let Some(dir) = cache_dir.as_deref() { @@ -131,8 +155,11 @@ pub fn transform_cached( "\n//# sourceMappingURL=data:application/json;base64,{b64}\n" )); } - // sourceURL magic comment — absolute file path, as Node's strip-types does. - code.push_str(&format!("\n//# sourceURL={filename}\n")); + // sourceURL magic comment — the `file:` URL, spelled as the host would. + code.push_str(&format!( + "\n//# sourceURL={}\n", + source_url::file_url(&filename, cfg!(windows), widened) + )); // Store: body = format_byte + code. if let Some(dir) = cache_dir.as_deref() { @@ -168,20 +195,21 @@ pub fn transform_cached( const BUILD_ID: &str = env!("NUB_NATIVE_BUILD_ID"); /// blake3(NUB_VERSION \0 SCHEMA \0 BUILD_ID \0 source \0 ext \0 tsconfig_hash \0 -/// pkg_type \0 filename) → 64-hex lowercase. blake3 (SIMD) replaces SHA-256 on the -/// hot path; the compile-time `BUILD_ID` is folded in so a rebuilt binary -/// auto-invalidates the cache without any runtime I/O. `filename` is keyed in -/// because the cached body bakes in a per-file `//# sourceURL` (issue #171), so -/// two byte-identical sources at different paths must not share an entry. The -/// derivation lives in the napi-free `nub-cache-key` crate so its invalidation -/// contract is unit-testable (this cdylib has `test = false` — a test harness -/// can't link the napi symbols). +/// pkg_type \0 filename \0 url_band) → 64-hex lowercase. blake3 (SIMD) replaces +/// SHA-256 on the hot path; the compile-time `BUILD_ID` is folded in so a rebuilt +/// binary auto-invalidates the cache without any runtime I/O. `filename` and +/// `url_band` are keyed in because the cached body bakes in a per-file +/// `//# sourceURL` (issue #171) whose spelling follows the host Node, so neither +/// two paths nor two hosts may share an entry. The derivation lives in the +/// napi-free `nub-cache-key` crate so its invalidation contract is unit-testable +/// (this cdylib has `test = false` — a test harness can't link the napi symbols). fn cache_key( source: &str, ext: &str, tsconfig_hash: &str, pkg_type: &str, filename: &str, + url_band: &str, ) -> String { nub_cache_key::cache_key( NUB_VERSION, @@ -192,6 +220,7 @@ fn cache_key( tsconfig_hash, pkg_type, filename, + url_band, ) } diff --git a/runtime/transform-core.mjs b/runtime/transform-core.mjs index 9ae4b891d..672480141 100644 --- a/runtime/transform-core.mjs +++ b/runtime/transform-core.mjs @@ -857,8 +857,12 @@ export function loadTranspile(url, ext) { tsconfig: RUNTIME_TSCONFIG || null, compilerOptions: RUNTIME_COMPILER_OPTIONS, }); + // process.version decides how the appended `//# sourceURL` is percent-encoded: + // it is spelled to match THIS host's pathToFileURL, whose escape set widened + // mid-release-line, so the same file has two valid spellings across hosts. + // Native derives the band from it and folds that band into the cache key. const result = nubNative.transformCached( - filePath, source, opts, ext, `${tsconfigHash || ""}\0${runtimeHash}`, pkgType || "", formatByte, getCacheDir() ?? undefined, + filePath, source, opts, ext, `${tsconfigHash || ""}\0${runtimeHash}`, pkgType || "", formatByte, getCacheDir() ?? undefined, process.version, ); if (result.errors.length > 0) { const details = result.errors.map((e) => e.codeframe || e.message).join("\n\n");