diff --git a/backend/tests/bun_jobs.rs b/backend/tests/bun_jobs.rs index e3e6ecdb2f..8facba4a6e 100644 --- a/backend/tests/bun_jobs.rs +++ b/backend/tests/bun_jobs.rs @@ -1284,7 +1284,8 @@ fn test_generate_bun_bundle_propagates_exit_status() { /// 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. +/// package as a namespace and read the names off `default` instead — while +/// leaving an ESM package's named imports as the live bindings node gives it. #[test] fn test_node_loader_cjs_named_export_interop() { use std::process::Command; @@ -1313,12 +1314,27 @@ module.exports = api; ) .unwrap(); + // An ESM package whose exported binding changes after evaluation. + let esm_dir = dir.join("node_modules").join("live-esm-pkg"); + std::fs::create_dir_all(&esm_dir).unwrap(); + std::fs::write( + esm_dir.join("package.json"), + r#"{ "name": "live-esm-pkg", "version": "1.0.0", "type": "module", "main": "index.js" }"#, + ) + .unwrap(); + std::fs::write( + esm_dir.join("index.js"), + "export let count = 0;\nexport function bump() { count++; }\n", + ) + .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")]; } +import { count, bump } from "live-esm-pkg"; +export function main() { bump(); return [greet("a"), pkg.greet("b"), count]; } "#, ) .unwrap(); @@ -1370,7 +1386,7 @@ console.log(JSON.stringify(Main.main())); "node rejected the bundle:\nstdout:\n{stdout}\nstderr:\n{}", String::from_utf8_lossy(&run.stderr) ); - assert_eq!(stdout.trim(), r#"["greet a","greet b"]"#); + assert_eq!(stdout.trim(), r#"["greet a","greet b",1]"#); } /// Regression test for the install_bun_lockfile no-DB path: same code shape as diff --git a/backend/windmill-worker/node_cjs_interop.js b/backend/windmill-worker/node_cjs_interop.js index cc2bd7e6d4..405a2faaf1 100644 --- a/backend/windmill-worker/node_cjs_interop.js +++ b/backend/windmill-worker/node_cjs_interop.js @@ -6,8 +6,12 @@ // 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. +// Only packages node loads as CommonJS are rewritten: the rewrite turns named +// imports into snapshots, which would freeze an ESM export its package mutates +// after evaluation. + +import { readFileSync } from "node:fs"; +import { dirname, join } from "node:path"; const WM_IDENT = "[A-Za-z_$][A-Za-z0-9_$]*"; const WM_NS_CLAUSE = `\\*\\s*as\\s+${WM_IDENT}`; @@ -15,12 +19,21 @@ 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) { +function wmRewriteExternalImports(code, externals, jobDir) { if (!externals || externals.length === 0) { return code; } - const isExternal = (spec) => - externals.some((name) => spec === name || spec.startsWith(name + "/")); + const eligible = new Map(); + const needsInterop = (spec) => { + let ok = eligible.get(spec); + if (ok === undefined) { + ok = + externals.some((name) => spec === name || spec.startsWith(name + "/")) && + wmIsCommonJs(spec, jobDir); + eligible.set(spec, ok); + } + return ok; + }; // 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. @@ -34,7 +47,7 @@ function wmRewriteExternalImports(code, externals) { continue; } const spec = m[3] !== undefined ? m[3] : m[4]; - if (isExternal(spec)) { + if (needsInterop(spec)) { found.push({ at: hit.index, len: hit[0].length, lead: m[1], clause: m[2], spec }); } } @@ -106,6 +119,38 @@ function wmRewriteExternalImports(code, externals) { ); } +// Node's own rule for the format of the file a specifier resolves to: the +// extension decides, and `.js` follows the `type` of the closest package.json. +function wmIsCommonJs(spec, jobDir) { + let file; + try { + file = Bun.resolveSync(spec, jobDir); + } catch (err) { + return true; + } + if (file.endsWith(".mjs")) { + return false; + } + if (!file.endsWith(".js")) { + return true; + } + let dir = dirname(file); + for (;;) { + let pkg = null; + try { + pkg = JSON.parse(readFileSync(join(dir, "package.json"), "utf8")); + } catch (err) {} + if (pkg !== null) { + return pkg.type !== "module"; + } + const parent = dirname(dir); + if (parent === dir) { + return true; + } + dir = parent; + } +} + function wmParseImportClause(clause) { let def = null; let ns = null; diff --git a/backend/windmill-worker/src/bun_executor.rs b/backend/windmill-worker/src/bun_executor.rs index 891ce37ff6..da694f37be 100644 --- a/backend/windmill-worker/src/bun_executor.rs +++ b/backend/windmill-worker/src/bun_executor.rs @@ -912,7 +912,7 @@ if (!result?.success || !(result.outputs?.length > 0)) {{ try {{ const bundlePath = "{job_dir_js}/wrapper.js"; const bundle = await Bun.file(bundlePath).text(); - const interoped = wmRewriteExternalImports(bundle, fileNames); + const interoped = wmRewriteExternalImports(bundle, fileNames, "{job_dir_js}"); if (interoped !== bundle) {{ await Bun.write(bundlePath, interoped); }}