Skip to content
Open
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/bun.js/bindings/JSCommonJSModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -812,6 +812,16 @@ class JSCommonJSModulePrototype final : public JSC::JSNonFinalObject {
clientData(vm)->builtinNames().requireNativeModulePrivateName(),
0,
jsFunctionRequireNativeModule, ImplementationVisibility::Public, NoIntrinsic, JSC::PropertyAttribute::ReadOnly | JSC::PropertyAttribute::DontDelete);

// `Module.prototype.load(filename)` — needed for packages like
// `requizzle` that construct `new Module(...)` directly and call
// `.load()` on it. Mirrors Node's cjs loader semantics.
this->putDirectBuiltinFunction(
vm,
globalObject,
JSC::Identifier::fromString(vm, "load"_s),
WebCore::commonJSModulePrototypeLoadCodeGenerator(vm),
0);
}
};

Expand Down
13 changes: 12 additions & 1 deletion src/bun.js/modules/NodeModuleModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -766,7 +766,7 @@ JSC_DEFINE_CUSTOM_SETTER(setNodeModuleWrapper,
static JSValue getModulePrototypeObject(VM& vm, JSObject* moduleObject)
{
auto* globalObject = defaultGlobalObject(moduleObject->globalObject());
auto prototype = constructEmptyObject(globalObject, globalObject->objectPrototype(), 2);
auto prototype = constructEmptyObject(globalObject, globalObject->objectPrototype(), 3);

prototype->putDirectCustomAccessor(
vm, WebCore::clientData(vm)->builtinNames().requirePublicName(),
Expand All @@ -776,6 +776,17 @@ static JSValue getModulePrototypeObject(VM& vm, JSObject* moduleObject)

prototype->putDirect(vm, Identifier::fromString(vm, "_compile"_s), globalObject->modulePrototypeUnderscoreCompileFunction());

// Also expose `load` here so `require('module').prototype.load` is a
// function, matching Node (whose `Module.prototype` IS the instance
// prototype and thus exposes both). The instance prototype
// (`JSCommonJSModulePrototype`) has its own `load` binding; this one
// is only consulted by code that reads `Module.prototype.load` off
// the constructor directly.
prototype->putDirect(
vm, Identifier::fromString(vm, "load"_s),
JSC::JSFunction::create(vm, globalObject, WebCore::commonJSModulePrototypeLoadCodeGenerator(vm), globalObject),
0);

return prototype;
}

Expand Down
64 changes: 64 additions & 0 deletions src/js/builtins/CommonJS.ts
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,70 @@
return proxy;
}

// `Module.prototype.load(filename)` — used by packages like `requizzle` that
// construct `new Module(...)` directly and expect Node's module-loader shape.
// Mirrors Node's lib/internal/modules/cjs/loader.js `Module.prototype.load`.
$visibility = "Private";
export function modulePrototypeLoad(this: JSCommonJSModule, filename: string) {

Check warning on line 423 in src/js/builtins/CommonJS.ts

View check run for this annotation

Claude / Claude Code Review

Module.prototype.load.name is "modulePrototypeLoad" not "load"

Module.prototype.load.name returns "modulePrototypeLoad" instead of "load" because the $overriddenName = "load" directive is missing before the function declaration in CommonJS.ts. JSC derives a builtin function's .name from the source declaration name, so both the instance prototype (JSCommonJSModulePrototype) and the disposable prototype (getModulePrototypeObject) will expose a load property whose .name === "modulePrototypeLoad". The fix is a one-liner: add $overriddenName = "load" immediately
// Match Node's `assert(!this.loaded, 'Module already loaded')` so a
// caller that catches the error and checks `e.code === 'ERR_ASSERTION'`
// behaves the same way on both runtimes.
const assert = require("node:assert");
assert(!this.loaded, "Module already loaded");

const Module = require("node:module");
const path = require("node:path");

// Update `filename`, `path` (= `m_dirname`, drives `__dirname`), and
// `paths` before dispatching: the .js handler goes through the native
// evaluate() path, which reads `this.path` for the module's __dirname.
// Without this, `__dirname` would stay at whatever the constructor was
// given, not where the file actually lives.
const dirname = path.dirname(filename);
this.filename = filename;
this.path = dirname;
this.paths = Module._nodeModulePaths(dirname);

// Find the longest-matching registered extension, mirroring Node's
// `findLongestRegisteredExtension` in lib/internal/modules/cjs/loader.js.
// `path.extname` only returns the trailing suffix, so it would miss
// compound extensions like `.test.js` or `.esm.js`.
const basename = path.basename(filename);
const extensions = Module._extensions;
let handler: any;
let startDot = basename.indexOf(".");
while (startDot !== -1 && startDot !== basename.length - 1) {
// Skip a leading dot so dotfiles like `.gitignore` don't match a
// handler registered for the full filename. Node's
// findLongestRegisteredExtension and Bun's native Zig equivalent
// both do this.
if (startDot === 0) {
startDot = basename.indexOf(".", 1);
continue;
}
const suffix = basename.slice(startDot);
handler = extensions[suffix];
if (handler) break;
startDot = basename.indexOf(".", startDot + 1);
}
if (!handler) {
handler = extensions[".js"];
}

// Don't let a throw from the handler leave the module permanently
// marked "loaded" — otherwise a retry would hit the assert above.
// `module._compile` sets `hasEvaluated=true` before running user code,
// which is what `loaded` reflects, so we reset it on failure.
try {
handler.$call(extensions, this, filename);
} catch (e) {
this.loaded = false;
throw e;
}

this.loaded = true;
}

type WrapperMutate = (start: string, end: string) => void;
export function getWrapperArrayProxy(onMutate: WrapperMutate) {
const wrapper = ["(function(exports,require,module,__filename,__dirname){", "})"];
Expand Down
258 changes: 258 additions & 0 deletions test/regression/issue/29253.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,258 @@
// https://github.com/oven-sh/bun/issues/29253
//
// `new Module(id, parent)` produced an instance whose prototype
// did not expose `Module.prototype.load(filename)`, so packages
// that construct a module by hand and then call `.load()` on it
// (the same pattern Node's internal cjs loader uses) threw:
//
// TypeError: targetModule.load is not a function
//
// `requizzle` — a dependency of `jsdoc` — does exactly this
// inside its `exports.load` helper, so `bun run .../jsdoc.js`
// crashed before jsdoc got a chance to run.
//
// The fix adds `Module.prototype.load` as a real method on the
// prototype backing `new Module(...)` instances, and also puts it
// on `require("module").prototype` so Node-compat property lookups
// see a function in both places. (The two objects are still
// distinct — full prototype unification is deferred because the
// existing `_compile` CustomAccessor depends on the instance cast.)
import { expect, test } from "bun:test";
import { bunEnv, bunExe, tempDir } from "harness";
import Module from "node:module";

test("new Module() instances inherit load() (#29253)", () => {
// The ticket: `targetModule.load(targetModule.id)` on a freshly
// constructed Module was throwing "load is not a function" because
// the instance prototype had no `load` method.
const m = new Module("/tmp/does-not-matter-29253.js", null);
expect(typeof m.load).toBe("function");

// And it should be inherited from the prototype chain, not an own
// property on every instance (which would be wasteful).
expect(Object.prototype.hasOwnProperty.call(m, "load")).toBe(false);
expect(typeof Object.getPrototypeOf(m).load).toBe("function");
});

test.concurrent("new Module().load(filename) reads and evaluates the file (#29253)", async () => {
// Spawn a separate Bun so the test doesn't pollute its own
// require cache or Module.wrap state.
using dir = tempDir("issue-29253-load", {
"target.js": `
module.exports = { answer: 42, filename: __filename, dirname: __dirname };
`,
"driver.js": `
const Module = require("node:module");
const path = require("node:path");
const target = path.resolve(__dirname, "target.js");

const m = new Module(target, module);
m.load(target);

// After load(): the file has been read, wrapped, and
// executed. The module's exports must be the object the
// file assigned to module.exports, and the bookkeeping
// fields must be populated the way Node does.
console.log(JSON.stringify({
loaded: m.loaded,
filename: m.filename,
exports: m.exports,
}));
`,
});

await using proc = Bun.spawn({
cmd: [bunExe(), "driver.js"],
env: bunEnv,
cwd: String(dir),
stdout: "pipe",
stderr: "pipe",
});

const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);

const result = JSON.parse(stdout.trim());
expect(result.loaded).toBe(true);
expect(result.filename).toMatch(/target\.js$/);
expect(result.exports.answer).toBe(42);
expect(result.exports.filename).toBe(result.filename);
expect(exitCode).toBe(0);
});

test.concurrent("Module.prototype.load honors an overridden Module.wrapper (#29253)", async () => {
// `load()` must compile the file through the CURRENT module
// wrapper (`Module.wrapper[0] + source + Module.wrapper[1]`)
// — not a hard-coded one. Mutating the wrapper array is how
// Bun exposes Node's wrapper-override hook.
using dir = tempDir("issue-29253-wrap", {
"target.js": `module.exports = { wrappedVar: typeof __swizzled };`,
"driver.js": `
const Module = require("node:module");
const path = require("node:path");
const originalWrapper0 = Module.wrapper[0];

// Inject a local 'const __swizzled = 1;' at the top of
// the module scope; if the wrapper is honored, the module
// sees typeof __swizzled === "number".
Module.wrapper[0] = originalWrapper0 + "const __swizzled = 1;\\n";

try {
const target = path.resolve(__dirname, "target.js");
const m = new Module(target, module);
m.load(target);
console.log(m.exports.wrappedVar);
} finally {
Module.wrapper[0] = originalWrapper0;
}
`,
});

await using proc = Bun.spawn({
cmd: [bunExe(), "driver.js"],
env: bunEnv,
cwd: String(dir),
stdout: "pipe",
stderr: "pipe",
});

const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);

expect(stdout.trim()).toBe("number");
expect(exitCode).toBe(0);
});

test.concurrent("new Module().load populates filename/paths/loaded (#29253)", async () => {
// Node's `Module.prototype.load` writes `filename`, `paths`,
// and `loaded` before returning. `requizzle` and any other
// package that reads those fields after `.load()` depends on
// this, even if it doesn't touch the wrapper.
using dir = tempDir("issue-29253-fields", {
"leaf.js": `module.exports = 'ok';`,
"driver.js": `
const Module = require("node:module");
const path = require("node:path");

const target = path.resolve(__dirname, "leaf.js");
const m = new Module(target, module);
// paths must come from Module._nodeModulePaths(dirname(filename)).
const expectedPaths = Module._nodeModulePaths(path.dirname(target));

// Pre-load state: loaded=false, no filename.
if (m.loaded !== false) throw new Error("pre-load 'loaded' should be false, got " + m.loaded);

m.load(target);

if (m.loaded !== true) throw new Error("post-load 'loaded' should be true");
if (m.filename !== target) throw new Error("filename mismatch: " + m.filename);
if (JSON.stringify(m.paths) !== JSON.stringify(expectedPaths)) {
throw new Error("paths mismatch: " + JSON.stringify(m.paths) + " vs " + JSON.stringify(expectedPaths));
}
if (m.exports !== 'ok') throw new Error("exports mismatch: " + m.exports);
console.log("ok");
`,
});

await using proc = Bun.spawn({
cmd: [bunExe(), "driver.js"],
env: bunEnv,
cwd: String(dir),
stdout: "pipe",
stderr: "pipe",
});

const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);

expect(stdout.trim()).toBe("ok");
expect(exitCode).toBe(0);
});

// Retry guard: a thrown extension handler must NOT leave the module
// permanently marked `loaded`, otherwise the next `.load(...)` call on
// the same instance would hit the "Module already loaded" assert and
// make failure recovery impossible.
test.concurrent("failed load() clears loaded so the instance can be retried (#29253)", async () => {
using dir = tempDir("issue-29253-retry", {
"broken.js": `throw new Error("boom");`,
"good.js": `module.exports = 'good-exports';`,
"driver.js": `
const Module = require("node:module");
const path = require("node:path");

const broken = path.resolve(__dirname, "broken.js");
const good = path.resolve(__dirname, "good.js");
const m = new Module(broken, module);

let threw = false;
try {
m.load(broken);
} catch (e) {
threw = true;
if (!String(e).includes("boom")) throw new Error("unexpected error: " + e);
}
if (!threw) throw new Error("expected load() to throw");
if (m.loaded) throw new Error("loaded should be false after a failed load()");

// Now reuse the instance with a good file — must not hit the
// "Module already loaded" guard.
m.load(good);
if (m.exports !== 'good-exports') throw new Error("retry exports mismatch: " + m.exports);
console.log("ok");
`,
});

await using proc = Bun.spawn({
cmd: [bunExe(), "driver.js"],
env: bunEnv,
cwd: String(dir),
stdout: "pipe",
stderr: "pipe",
});

const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);

expect(stdout.trim()).toBe("ok");
expect(exitCode).toBe(0);
});

// Compound-extension dispatch: `Module._extensions['.test.js']` must win
// over `Module._extensions['.js']` when `.load()` is called on a file
// ending in `.test.js`. `path.extname` alone would return `.js` and
// silently bypass the compound handler.
test.concurrent("load() picks the longest registered extension handler (#29253)", async () => {
using dir = tempDir("issue-29253-ext", {
"foo.test.js": `module.exports = 'raw-source-never-loaded';`,
"driver.js": `
const Module = require("node:module");
const path = require("node:path");

const target = path.resolve(__dirname, "foo.test.js");
Module._extensions['.test.js'] = function (module, filename) {
module.exports = { hookedBy: '.test.js', filename };
};

try {
const m = new Module(target, module);
m.load(target);
if (m.exports.hookedBy !== '.test.js') {
throw new Error("handler not used; exports=" + JSON.stringify(m.exports));
}
} finally {
delete Module._extensions['.test.js'];
}
console.log("ok");
`,
});

await using proc = Bun.spawn({
cmd: [bunExe(), "driver.js"],
env: bunEnv,
cwd: String(dir),
stdout: "pipe",
stderr: "pipe",
});

const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);

expect(stdout.trim()).toBe("ok");
expect(exitCode).toBe(0);
});
Loading