mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
fix: classify //nodejs interop targets with node's resolver and merge namespace keys
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014bq66fSmaPvLtjnDPfMYcj
This commit is contained in:
Ruben Fiszel
co-authored by
Claude Opus 5
Claude Opus 5
parent
26102edd89
commit
1a150736e2
+59
-28
@@ -1284,8 +1284,10 @@ 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 — while
|
||||
/// leaving an ESM package's named imports as the live bindings node gives it.
|
||||
/// package as a namespace and read the names off `default` instead, while
|
||||
/// leaving alone anything node loads as ESM — including a package whose
|
||||
/// conditional exports hand bun a different file than they hand node — so its
|
||||
/// named imports stay the live bindings node gives them.
|
||||
#[test]
|
||||
fn test_node_loader_cjs_named_export_interop() {
|
||||
use std::process::Command;
|
||||
@@ -1294,39 +1296,63 @@ fn test_node_loader_cjs_named_export_interop() {
|
||||
let temp_dir = tempfile::tempdir().unwrap();
|
||||
let dir = temp_dir.path();
|
||||
let dir_str = dir.to_str().unwrap();
|
||||
let write_pkg = |name: &str, files: &[(&str, &str)]| {
|
||||
let pkg_dir = dir.join("node_modules").join(name);
|
||||
std::fs::create_dir_all(&pkg_dir).unwrap();
|
||||
for (file, content) in files {
|
||||
std::fs::write(pkg_dir.join(file), content).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#"
|
||||
write_pkg(
|
||||
"dyn-cjs-pkg",
|
||||
&[
|
||||
(
|
||||
"package.json",
|
||||
r#"{ "name": "dyn-cjs-pkg", "version": "1.0.0", "main": "index.js" }"#,
|
||||
),
|
||||
(
|
||||
"index.js",
|
||||
r#"
|
||||
const api = {};
|
||||
["greet"].forEach((k) => { api[k] = (s) => k + " " + s; });
|
||||
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();
|
||||
write_pkg(
|
||||
"live-esm-pkg",
|
||||
&[
|
||||
(
|
||||
"package.json",
|
||||
r#"{ "name": "live-esm-pkg", "version": "1.0.0", "type": "module", "main": "index.js" }"#,
|
||||
),
|
||||
("index.js", "export let count = 0;\nexport function bump() { count++; }\n"),
|
||||
],
|
||||
);
|
||||
|
||||
// Conditional exports that give bun CommonJS and node ESM: only node's
|
||||
// answer says whether the bundle may snapshot the bindings.
|
||||
write_pkg(
|
||||
"dual-cond-pkg",
|
||||
&[
|
||||
(
|
||||
"package.json",
|
||||
r#"{ "name": "dual-cond-pkg", "version": "1.0.0",
|
||||
"exports": { ".": { "bun": "./bun-cjs.js", "node": "./node-esm.mjs", "default": "./node-esm.mjs" } } }"#,
|
||||
),
|
||||
("bun-cjs.js", "module.exports = { count: 0, bump() {} };\n"),
|
||||
(
|
||||
"node-esm.mjs",
|
||||
"export let count = 0;\nexport function bump() { count++; }\n",
|
||||
),
|
||||
],
|
||||
);
|
||||
|
||||
std::fs::write(
|
||||
dir.join("main.ts"),
|
||||
@@ -1334,7 +1360,12 @@ module.exports = api;
|
||||
import { greet } from "dyn-cjs-pkg";
|
||||
import * as pkg from "dyn-cjs-pkg";
|
||||
import { count, bump } from "live-esm-pkg";
|
||||
export function main() { bump(); return [greet("a"), pkg.greet("b"), count]; }
|
||||
import { count as dual, bump as bumpDual } from "dual-cond-pkg";
|
||||
export function main() {
|
||||
bump();
|
||||
bumpDual();
|
||||
return [greet("a"), pkg.greet("b"), { ...pkg }.greet("c"), count, dual];
|
||||
}
|
||||
"#,
|
||||
)
|
||||
.unwrap();
|
||||
@@ -1386,7 +1417,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",1]"#);
|
||||
assert_eq!(stdout.trim(), r#"["greet a","greet b","greet c",1,1]"#);
|
||||
}
|
||||
|
||||
/// Regression test for the install_bun_lockfile no-DB path: same code shape as
|
||||
|
||||
@@ -12,6 +12,7 @@
|
||||
|
||||
import { readFileSync } from "node:fs";
|
||||
import { dirname, join } from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
|
||||
const WM_IDENT = "[A-Za-z_$][A-Za-z0-9_$]*";
|
||||
const WM_NS_CLAUSE = `\\*\\s*as\\s+${WM_IDENT}`;
|
||||
@@ -20,21 +21,12 @@ const WM_CLAUSE = `(?:${WM_NS_CLAUSE}|${WM_NAMED_CLAUSE}|${WM_IDENT}(?:\\s*,\\s*
|
||||
const WM_ATTRS = "\\s*(?:with|assert)\\s*\\{[^{}]*\\}";
|
||||
const WM_IMPORT = `(^|[;}\\n])import\\s*(?:(${WM_CLAUSE})\\s*from\\s*)?(?:"([^"\\n]*)"|'([^'\\n]*)')(${WM_ATTRS})?`;
|
||||
|
||||
function wmRewriteExternalImports(code, externals, jobDir) {
|
||||
function wmRewriteExternalImports(code, externals, jobDir, nodePath) {
|
||||
if (!externals || externals.length === 0) {
|
||||
return code;
|
||||
}
|
||||
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;
|
||||
};
|
||||
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.
|
||||
@@ -43,15 +35,15 @@ function wmRewriteExternalImports(code, externals, jobDir) {
|
||||
// makes the rewrite safe — it is load-bearing, not a belt-and-braces extra.
|
||||
const masked = wmMaskLiterals(code);
|
||||
const anchored = new RegExp(WM_IMPORT);
|
||||
const found = [];
|
||||
const matches = [];
|
||||
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 (needsInterop(spec)) {
|
||||
found.push({
|
||||
if (isExternal(spec)) {
|
||||
matches.push({
|
||||
at: hit.index,
|
||||
len: hit[0].length,
|
||||
lead: m[1],
|
||||
@@ -61,6 +53,16 @@ function wmRewriteExternalImports(code, externals, jobDir) {
|
||||
});
|
||||
}
|
||||
}
|
||||
if (matches.length === 0) {
|
||||
return code;
|
||||
}
|
||||
|
||||
const commonjs = wmCommonJsSpecs(
|
||||
[...new Set(matches.map((m) => m.spec))],
|
||||
jobDir,
|
||||
nodePath
|
||||
);
|
||||
const found = matches.filter((m) => commonjs.has(m.spec));
|
||||
if (found.length === 0) {
|
||||
return code;
|
||||
}
|
||||
@@ -125,22 +127,55 @@ function wmRewriteExternalImports(code, externals, jobDir) {
|
||||
return (
|
||||
`var ${getHelper}=(n,k,s)=>{if(k in n)return n[k];let d=Object(n.default);if(k in d)return d[k];` +
|
||||
"throw new SyntaxError(`The requested module '${s}' does not provide an export named '${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};` +
|
||||
`var ${nsHelper}=(n)=>{let d=n.default;if(d==null||typeof d!=="object"&&typeof d!=="function")return n;` +
|
||||
`let t={},a=(o,k)=>Object.defineProperty(t,k,{get:()=>o[k],enumerable:!0,configurable:!0});` +
|
||||
`for(let k of Object.keys(d))a(d,k);for(let k of Object.keys(n))a(n,k);return t};` +
|
||||
out +
|
||||
code.slice(cursor)
|
||||
);
|
||||
}
|
||||
|
||||
// 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;
|
||||
// Node runs the bundle, and conditional exports can hand it a different file
|
||||
// than they hand bun, so ask node itself where each specifier resolves. Bun's
|
||||
// resolver is only the fallback for a node too old for `import.meta.resolve`.
|
||||
function wmCommonJsSpecs(specs, jobDir, nodePath) {
|
||||
const commonjs = new Set();
|
||||
let resolved = null;
|
||||
const probe =
|
||||
"const out={};" +
|
||||
"for(const s of JSON.parse(process.argv[1])){try{out[s]=import.meta.resolve(s)}catch(e){out[s]=null}}" +
|
||||
"console.log(JSON.stringify(out))";
|
||||
try {
|
||||
file = Bun.resolveSync(spec, jobDir);
|
||||
} catch (err) {
|
||||
return true;
|
||||
const run = Bun.spawnSync([nodePath, "--input-type=module", "-e", probe, JSON.stringify(specs)], {
|
||||
cwd: jobDir,
|
||||
});
|
||||
if (run.success) {
|
||||
resolved = JSON.parse(run.stdout.toString());
|
||||
}
|
||||
} catch (err) {}
|
||||
for (const spec of specs) {
|
||||
let file;
|
||||
if (resolved === null) {
|
||||
try {
|
||||
file = Bun.resolveSync(spec, jobDir);
|
||||
} catch (err) {
|
||||
continue;
|
||||
}
|
||||
} else if (resolved[spec] != null) {
|
||||
file = fileURLToPath(resolved[spec]);
|
||||
} else {
|
||||
continue;
|
||||
}
|
||||
if (wmIsCommonJsFile(file)) {
|
||||
commonjs.add(spec);
|
||||
}
|
||||
}
|
||||
return commonjs;
|
||||
}
|
||||
|
||||
// Node's own rule for a file's format: the extension decides, and `.js` follows
|
||||
// the `type` of the closest package.json.
|
||||
function wmIsCommonJsFile(file) {
|
||||
if (file.endsWith(".mjs")) {
|
||||
return false;
|
||||
}
|
||||
|
||||
@@ -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, "{job_dir_js}");
|
||||
const interoped = wmRewriteExternalImports(bundle, fileNames, "{job_dir_js}", {node_bin});
|
||||
if (interoped !== bundle) {{
|
||||
await Bun.write(bundlePath, interoped);
|
||||
}}
|
||||
@@ -920,7 +920,9 @@ try {{
|
||||
console.log("Failed to apply CommonJS interop to the node bundle: " + err);
|
||||
}}
|
||||
"#,
|
||||
interop = NODE_CJS_INTEROP
|
||||
interop = NODE_CJS_INTEROP,
|
||||
node_bin = serde_json::to_string(&*NODE_BIN_PATH)
|
||||
.unwrap_or_else(|_| "\"node\"".to_string())
|
||||
),
|
||||
)?;
|
||||
} else if mode == LoaderMode::Bun {
|
||||
|
||||
Reference in New Issue
Block a user