Skip to content
Open
Show file tree
Hide file tree
Changes from 2 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
27 changes: 18 additions & 9 deletions src/bun.js/modules/NodeModuleModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -765,16 +765,25 @@ JSC_DEFINE_CUSTOM_SETTER(setNodeModuleWrapper,

static JSValue getModulePrototypeObject(VM& vm, JSObject* moduleObject)
{
// `Module.prototype` must be the SAME object that sits in the prototype
// chain of instances created via `new Module(...)`. Otherwise, assigning
// `Module.prototype.foo = ...` won't affect instances, and methods added
// to the real instance prototype (e.g. `load`) won't be visible via
// `Module.prototype`. Packages like `requizzle` depend on this.
auto* globalObject = defaultGlobalObject(moduleObject->globalObject());
auto prototype = constructEmptyObject(globalObject, globalObject->objectPrototype(), 2);

prototype->putDirectCustomAccessor(
vm, WebCore::clientData(vm)->builtinNames().requirePublicName(),
JSC::CustomGetterSetter::create(vm, getterRequireFunction,
setterRequireFunction),
0);

prototype->putDirect(vm, Identifier::fromString(vm, "_compile"_s), globalObject->modulePrototypeUnderscoreCompileFunction());
auto* structure = globalObject->CommonJSModuleObjectStructure();
auto* prototype = structure->storedPrototypeObject();

// Expose `require` on the public prototype. Instance-level `require` is
// set per-instance during module evaluation, so the prototype accessor
// is only hit for instances constructed outside the normal loader path.
if (!prototype->getDirect(vm, WebCore::clientData(vm)->builtinNames().requirePublicName())) {
prototype->putDirectCustomAccessor(
vm, WebCore::clientData(vm)->builtinNames().requirePublicName(),
JSC::CustomGetterSetter::create(vm, getterRequireFunction,
setterRequireFunction),
0);
}

return prototype;
}
Expand Down
33 changes: 33 additions & 0 deletions src/js/builtins/CommonJS.ts
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,40 @@

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";

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

View check run for this annotation

Claude / Claude Code Review

modulePrototypeLoad throws plain Error instead of AssertionError for already-loaded module

In modulePrototypeLoad, the already-loaded guard throws new Error('Module already loaded') instead of an AssertionError as Node.js does. Code checking instanceof AssertionError or e.code === 'ERR_ASSERTION' on this error will behave differently between Node.js and Bun. This is a narrow compatibility gap: calling .load() on an already-loaded module is itself a programming error and no real-world code inspects the error type here.
export function modulePrototypeLoad(this: JSCommonJSModule, filename: string) {
if (this.loaded) {
throw new Error("Module already loaded");
}

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

Check failure on line 430 in src/js/builtins/CommonJS.ts

View workflow job for this annotation

GitHub Actions / Lint JavaScript

eslint(no-unused-vars)

Variable 'fs' is declared but never used. Unused variables should start with a '_'.

this.filename = filename;
this.paths = Module._nodeModulePaths(path.dirname(filename));

const ext = path.extname(filename);
const extensions = Module._extensions;
const handler = extensions[ext] || extensions[".js"];
handler.$call(extensions, this, filename);

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

View check run for this annotation

Claude / Claude Code Review

modulePrototypeLoad uses path.extname instead of findLongestRegisteredExtension

In `modulePrototypeLoad` (CommonJS.ts:436), the extension lookup uses `path.extname(filename)` which only returns the last extension component — `.js` for `foo.test.js` — so a user-registered `Module._extensions['.test.js']` handler is never consulted. Node.js and Bun's own native require path both use `findLongestRegisteredExtension` which tries progressively shorter suffixes (e.g. `.test.js` before `.js`); the new TypeScript implementation breaks that contract.

this.loaded = true;
}

Check failure on line 441 in src/js/builtins/CommonJS.ts

View check run for this annotation

Claude / Claude Code Review

module.loaded stuck at true after a failed load(), blocking retry

After a failed `new Module(id).load(id)` call, `module.loaded` returns `true` even though execution never completed, permanently blocking any retry with "Module already loaded". In Node.js, `this.loaded` is only set after the extension handler returns successfully, so the same module object can be retried on failure.

// `Module._extensions['.js']` — reads file, runs it through `module._compile`
// (which honors the current (possibly overridden) `Module.wrap`).
$visibility = "Private";
export function modulePrototypeLoadJSExtension(this: any, module: JSCommonJSModule, filename: string) {
const fs = require("node:fs");
const content = fs.readFileSync(filename, "utf8");
module._compile(content, filename);
}

type WrapperMutate = (start: string, end: string) => void;
export function getWrapperArrayProxy(onMutate: WrapperMutate) {
const wrapper = ["(function(exports,require,module,__filename,__dirname){", "})"];
Expand Down
173 changes: 173 additions & 0 deletions test/regression/issue/29253.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,173 @@
// 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 shared by instances created via `new Module(...)`
// and unifies `require("module").prototype` with that same
// prototype, so patching one is reflected in the other (Node
// semantics).
import { expect, test } from "bun:test";
import { bunEnv, bunExe, tempDir } from "harness";
import Module from "node:module";

test("Module.prototype.load is a function (#29253)", () => {
// The one the ticket is about: the stub on the instance prototype.
expect(typeof Module.prototype.load).toBe("function");

// An instance created via `new Module(...)` must inherit `.load`.
const m = new Module("/tmp/does-not-matter-29253.js", null);
expect(typeof m.load).toBe("function");
});

test("Module.prototype is the instance prototype (#29253)", () => {
// Node guarantees these are the same object — so patching
// `Module.prototype.foo` is visible on every instance. Several
// libraries (next.js, requizzle, etc.) rely on this.
const m = new Module("/tmp/does-not-matter-29253-proto.js", null);
expect(Object.getPrototypeOf(m)).toBe(Module.prototype);
});

test("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]);

expect(stderr).not.toContain("TypeError");
expect(stderr).not.toContain("Error");
expect(exitCode).toBe(0);

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);
});

test("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(stderr).not.toContain("TypeError");
expect(stderr).not.toContain("ReferenceError");
expect(exitCode).toBe(0);
expect(stdout.trim()).toBe("number");
});

test("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);

// 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 (!Array.isArray(m.paths)) throw new Error("paths should be an array");
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(stderr).not.toContain("TypeError");
expect(stderr).not.toContain("Error:");
expect(exitCode).toBe(0);
expect(stdout.trim()).toBe("ok");
});
Loading