From 0612abd21d7232db6a6cc1c85d7eea7836e83f56 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Thu, 20 Aug 2026 21:21:20 +0000 Subject: [PATCH] fix: make named and namespace npm imports work under //nodejs Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014bq66fSmaPvLtjnDPfMYcj --- backend/tests/bun_jobs.rs | 94 ++++++++ backend/windmill-worker/node_cjs_interop.js | 235 ++++++++++++++++++++ backend/windmill-worker/src/bun_executor.rs | 17 +- 3 files changed, 345 insertions(+), 1 deletion(-) create mode 100644 backend/windmill-worker/node_cjs_interop.js diff --git a/backend/tests/bun_jobs.rs b/backend/tests/bun_jobs.rs index fc766253bc..e3e6ecdb2f 100644 --- a/backend/tests/bun_jobs.rs +++ b/backend/tests/bun_jobs.rs @@ -1279,6 +1279,100 @@ fn test_generate_bun_bundle_propagates_exit_status() { ); } +/// `//nodejs` bundles keep the packages installed under `node_modules` external, +/// so node — not bun — resolves them at runtime. Node's cjs-module-lexer cannot +/// see the named exports of a CommonJS package that builds `module.exports` +/// dynamically (lodash & co.), so a named import fails to instantiate and a +/// namespace import yields nothing but `default`. The bundle must import such a +/// package as a namespace and read the names off `default` instead. +#[test] +fn test_node_loader_cjs_named_export_interop() { + use std::process::Command; + use windmill_worker::{build_loader, LoaderMode, BUN_PATH, NODE_BIN_PATH}; + + let temp_dir = tempfile::tempdir().unwrap(); + let dir = temp_dir.path(); + let dir_str = dir.to_str().unwrap(); + + // A CommonJS package whose exports only exist once it has run, which is + // what defeats the lexer. + let pkg_dir = dir.join("node_modules").join("dyn-cjs-pkg"); + std::fs::create_dir_all(&pkg_dir).unwrap(); + std::fs::write( + pkg_dir.join("package.json"), + r#"{ "name": "dyn-cjs-pkg", "version": "1.0.0", "main": "index.js" }"#, + ) + .unwrap(); + std::fs::write( + pkg_dir.join("index.js"), + r#" +const api = {}; +["greet"].forEach((k) => { api[k] = (s) => k + " " + s; }); +module.exports = api; +"#, + ) + .unwrap(); + + std::fs::write( + dir.join("main.ts"), + r#" +import { greet } from "dyn-cjs-pkg"; +import * as pkg from "dyn-cjs-pkg"; +export function main() { return [greet("a"), pkg.greet("b")]; } +"#, + ) + .unwrap(); + std::fs::write( + dir.join("wrapper.mjs"), + r#" +import * as Main from "./main.ts"; +console.log(JSON.stringify(Main.main())); +"#, + ) + .unwrap(); + + tokio::runtime::Runtime::new() + .unwrap() + .block_on(build_loader( + dir_str, + "http://localhost:8000", + "test_token", + "test-workspace", + "f/test/script", + LoaderMode::Node, + &None, + )) + .expect("build_loader failed"); + + let build = Command::new(BUN_PATH.as_str()) + .args(["run", "node_builder.ts"]) + .current_dir(dir) + .output() + .expect("Failed to run bun"); + assert!( + build.status.success(), + "node_builder.ts failed:\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&build.stdout), + String::from_utf8_lossy(&build.stderr) + ); + + // Same hand-off as generate_wrapper_mjs: the bundle replaces its entrypoint. + std::fs::rename(dir.join("wrapper.js"), dir.join("wrapper.mjs")).unwrap(); + + let run = Command::new(NODE_BIN_PATH.as_str()) + .arg("wrapper.mjs") + .current_dir(dir) + .output() + .expect("Failed to run node"); + let stdout = String::from_utf8_lossy(&run.stdout); + assert!( + run.status.success(), + "node rejected the bundle:\nstdout:\n{stdout}\nstderr:\n{}", + String::from_utf8_lossy(&run.stderr) + ); + assert_eq!(stdout.trim(), r#"["greet a","greet b"]"#); +} + /// Regression test for the install_bun_lockfile no-DB path: same code shape as /// `generate_bun_bundle` (site 3 of the original bug) — `wait().await?` ignored /// non-zero bun exits. A `bun install` failure (e.g. malformed package.json) diff --git a/backend/windmill-worker/node_cjs_interop.js b/backend/windmill-worker/node_cjs_interop.js new file mode 100644 index 0000000000..cc2bd7e6d4 --- /dev/null +++ b/backend/windmill-worker/node_cjs_interop.js @@ -0,0 +1,235 @@ +// Node's ESM loader only exposes the named exports of a CommonJS package that +// cjs-module-lexer can detect statically. lodash & co. build `module.exports` at +// runtime, so `import { chunk } from "lodash"` fails at instantiation and +// `import * as _ from "lodash"` yields a namespace holding nothing but `default`. +// npm packages stay external to the bundle, so Bun emits those import statements +// verbatim and node hits the limitation; rewrite them to a namespace import plus +// a lookup that falls back to `default` (i.e. `module.exports`). +// +// Named bindings become snapshots instead of live bindings, which only differs +// for an ESM package that mutates an exported binding after evaluation. + +const WM_IDENT = "[A-Za-z_$][A-Za-z0-9_$]*"; +const WM_NS_CLAUSE = `\\*\\s*as\\s+${WM_IDENT}`; +const WM_NAMED_CLAUSE = "\\{[^{}]*\\}"; +const WM_CLAUSE = `(?:${WM_NS_CLAUSE}|${WM_NAMED_CLAUSE}|${WM_IDENT}(?:\\s*,\\s*(?:${WM_NS_CLAUSE}|${WM_NAMED_CLAUSE}))?)`; +const WM_IMPORT = `(^|[;}\\n])import\\s*(?:(${WM_CLAUSE})\\s*from\\s*)?(?:"([^"\\n]*)"|'([^'\\n]*)')`; + +function wmRewriteExternalImports(code, externals) { + if (!externals || externals.length === 0) { + return code; + } + const isExternal = (spec) => + externals.some((name) => spec === name || spec.startsWith(name + "/")); + + // Import statements are found on a copy whose literals are blanked out, so + // that generated code holding an import statement in a string is not touched. + // Blanking preserves offsets and can only hide matches, never invent one. + const masked = wmMaskLiterals(code); + const anchored = new RegExp(WM_IMPORT); + const found = []; + for (const hit of masked.matchAll(new RegExp(WM_IMPORT, "g"))) { + const m = code.slice(hit.index, hit.index + hit[0].length).match(anchored); + if (m === null || m.index !== 0) { + continue; + } + const spec = m[3] !== undefined ? m[3] : m[4]; + if (isExternal(spec)) { + found.push({ at: hit.index, len: hit[0].length, lead: m[1], clause: m[2], spec }); + } + } + if (found.length === 0) { + return code; + } + + // Backstop for a literal the masking missed: if the statements found are not + // the ones the parser sees, leave the bundle alone rather than corrupt it. + const counts = new Map(); + for (const f of found) { + counts.set(f.spec, (counts.get(f.spec) ?? 0) + 1); + } + for (const imp of new Bun.Transpiler({ loader: "js" }).scanImports(code)) { + if (imp.kind === "import-statement" && counts.has(imp.path)) { + counts.set(imp.path, counts.get(imp.path) - 1); + } + } + for (const [spec, left] of counts) { + if (left !== 0) { + console.log( + `Skipping CommonJS interop rewrite of "${spec}": import statements found do not match the parsed ones` + ); + return code; + } + } + + let prefix = "__wm_ext"; + while (code.includes(prefix)) { + prefix += "_"; + } + const getHelper = `${prefix}Get`; + const nsHelper = `${prefix}Ns`; + let out = ""; + let cursor = 0; + let count = 0; + for (const f of found) { + const parts = f.clause === undefined ? null : wmParseImportClause(f.clause); + // A default-only import already resolves to `module.exports` under node. + if (parts === null || (parts.named.length === 0 && parts.ns === null)) { + continue; + } + const ns = `${prefix}${count++}`; + const decls = []; + if (parts.def !== null) { + decls.push(`${parts.def}=${ns}.default`); + } + if (parts.ns !== null) { + decls.push(`${parts.ns}=${nsHelper}(${ns})`); + } + for (const [imported, local] of parts.named) { + decls.push(`${local}=${getHelper}(${ns},${JSON.stringify(imported)})`); + } + out += + code.slice(cursor, f.at) + + `${f.lead}import*as ${ns} from${JSON.stringify(f.spec)};const ${decls.join(",")};`; + cursor = f.at + f.len; + } + if (count === 0) { + return code; + } + + return ( + `var ${getHelper}=(n,k)=>k in n?n[k]:n.default==null?undefined:n.default[k];` + + `var ${nsHelper}=(n)=>{let d=n.default;return d!=null&&(typeof d==="object"||typeof d==="function")` + + `?new Proxy(n,{get:(t,k,r)=>k in t?Reflect.get(t,k,r):d[k]}):n};` + + out + + code.slice(cursor) + ); +} + +function wmParseImportClause(clause) { + let def = null; + let ns = null; + const named = []; + let rest = clause.trim(); + + if (!rest.startsWith("{") && !rest.startsWith("*")) { + const m = rest.match(new RegExp(`^(${WM_IDENT})\\s*(?:,([\\s\\S]*))?$`)); + if (m === null) { + return null; + } + def = m[1]; + rest = (m[2] ?? "").trim(); + } + + if (rest.startsWith("*")) { + const m = rest.match(new RegExp(`^\\*\\s*as\\s+(${WM_IDENT})$`)); + if (m === null) { + return null; + } + ns = m[1]; + } else if (rest.startsWith("{")) { + const specifier = new RegExp( + `^(${WM_IDENT}|"[^"]*"|'[^']*')(?:\\s+as\\s+(${WM_IDENT}))?$` + ); + for (const part of rest.slice(1, rest.lastIndexOf("}")).split(",")) { + const trimmed = part.trim(); + if (trimmed === "") { + continue; + } + const m = trimmed.match(specifier); + if (m === null) { + return null; + } + const quoted = m[1].startsWith('"') || m[1].startsWith("'"); + if (quoted && m[2] === undefined) { + return null; + } + named.push([quoted ? m[1].slice(1, -1) : m[1], m[2] ?? m[1]]); + } + } else if (rest !== "") { + return null; + } + + return { def, ns, named }; +} + +// Replaces the contents of every string, template, comment and regex literal +// with `x`, keeping the delimiters, every newline and the total length. +function wmMaskLiterals(code) { + let out = ""; + let i = 0; + let cursor = 0; + let prev = ""; + const blank = (from, to) => { + out += code.slice(cursor, from); + for (const c of code.slice(from, to)) { + out += c === "\n" ? "\n" : "x"; + } + cursor = to; + }; + while (i < code.length) { + const c = code[i]; + if (c === '"' || c === "'" || c === "`") { + const start = ++i; + while (i < code.length) { + if (code[i] === "\\") { + i += 2; + } else if (code[i] === c) { + break; + } else { + i++; + } + } + blank(start, Math.min(i, code.length)); + i++; + prev = "'"; + continue; + } + if (c === "/" && code[i + 1] === "/") { + const start = i + 2; + while (i < code.length && code[i] !== "\n") { + i++; + } + blank(start, i); + continue; + } + if (c === "/" && code[i + 1] === "*") { + const end = code.indexOf("*/", i + 2); + const start = i + 2; + i = end === -1 ? code.length : end + 2; + blank(start, Math.max(start, i - 2)); + continue; + } + // `/` only starts a regex where an operand cannot stand; a wrong guess can + // only make the masking hide more than it should, which the parser + // cross-check catches. + if (c === "/" && !/[A-Za-z0-9_$)\]]/.test(prev)) { + const start = ++i; + let inClass = false; + while (i < code.length) { + if (code[i] === "\\") { + i += 2; + } else if (code[i] === "[") { + inClass = true; + i++; + } else if (code[i] === "]") { + inClass = false; + i++; + } else if (code[i] === "\n" || (code[i] === "/" && !inClass)) { + break; + } else { + i++; + } + } + blank(start, Math.min(i, code.length)); + i++; + prev = "'"; + continue; + } + if (!/\s/.test(c)) { + prev = c; + } + i++; + } + return out + code.slice(cursor); +} diff --git a/backend/windmill-worker/src/bun_executor.rs b/backend/windmill-worker/src/bun_executor.rs index e5134e8a70..891ce37ff6 100644 --- a/backend/windmill-worker/src/bun_executor.rs +++ b/backend/windmill-worker/src/bun_executor.rs @@ -71,6 +71,8 @@ pub const RELATIVE_BUN_LOADER: &str = include_str!("../loader.bun.windows.js"); pub const RELATIVE_BUN_BUILDER: &str = include_str!("../loader_builder.bun.js"); +pub const NODE_CJS_INTEROP: &str = include_str!("../node_cjs_interop.js"); + const NSJAIL_CONFIG_RUN_BUN_CONTENT: &str = include_str!("../nsjail/run.bun.config.proto"); pub const BUN_LOCK_SPLIT: &str = "\n//bun.lock\n"; @@ -877,6 +879,8 @@ pub async fn build_loader( r#" {loader} +{interop} + import {{ readdir }} from "node:fs/promises"; let fileNames = [] @@ -905,7 +909,18 @@ if (!result?.success || !(result.outputs?.length > 0)) {{ console.log("Failed to build node bundle: success=" + result?.success + ", outputs=" + (result?.outputs?.length ?? 0)); process.exit(1); }} -"# +try {{ + const bundlePath = "{job_dir_js}/wrapper.js"; + const bundle = await Bun.file(bundlePath).text(); + const interoped = wmRewriteExternalImports(bundle, fileNames); + if (interoped !== bundle) {{ + await Bun.write(bundlePath, interoped); + }} +}} catch(err) {{ + console.log("Failed to apply CommonJS interop to the node bundle: " + err); +}} +"#, + interop = NODE_CJS_INTEROP ), )?; } else if mode == LoaderMode::Bun {