Skip to content
52 changes: 52 additions & 0 deletions src/jsc/bindings/ModuleLoader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -919,6 +919,37 @@

extern "C" bool isBunTest;

// JSON/TOML/text-like modules are pure data: the ESM default export and the
// CommonJS `module.exports` are the same parsed value. Share that value with
// require.cache so `import default from "./x.json"` and `require("./x.json")`
// observe one object (Node.js guarantees this identity for JSON).
Comment thread
robobun marked this conversation as resolved.
Outdated
static JSC::JSValue reconcileDataModuleWithRequireCache(
Zig::GlobalObject* globalObject,
JSC::JSString* specifierJS,
JSC::JSValue value)
{
auto& vm = JSC::getVM(globalObject);
auto scope = DECLARE_THROW_SCOPE(vm);
JSValue entry = globalObject->requireMap()->get(globalObject, specifierJS);
RETURN_IF_EXCEPTION(scope, {});
if (auto* mod = entry ? dynamicDowncast<Bun::JSCommonJSModule>(entry) : nullptr) {
if (mod->hasEvaluated) {
JSValue existing = mod->exportsObject();
RETURN_IF_EXCEPTION(scope, {});
if (existing)
return existing;
}
mod->setExportsObject(value);
mod->hasEvaluated = true;
return value;
Comment thread
robobun marked this conversation as resolved.
Outdated
}
auto* mod = Bun::JSCommonJSModule::create(globalObject, specifierJS, value, true, jsUndefined());
RETURN_IF_EXCEPTION(scope, {});
globalObject->requireMap()->set(globalObject, specifierJS, mod);
RETURN_IF_EXCEPTION(scope, {});

Check warning on line 949 in src/jsc/bindings/ModuleLoader.cpp

View check run for this annotation

Claude / Claude Code Review

ESM data-module import overwrites non-JSCommonJSModule require.cache entries

`reconcileDataModuleWithRequireCache` overwrites a user-installed plain-object `require.cache` entry: `JSMap::get()` returns `jsUndefined()` for a missing key, so `dynamicDowncast<JSCommonJSModule>(entry)` yields `nullptr` for both "no entry" and "entry is a plain `{exports: mock}`", and both fall through to `create()` + `requireMap()->set()`. Pre-PR the ESM JSON/TOML path never touched `requireMap`, so a plain-object mock survived and `overridableRequire` returned `existing.exports`; post-PR th
Comment thread
robobun marked this conversation as resolved.
return value;
}

template<bool allowPromise>
static JSValue fetchESMSourceCode(
Zig::GlobalObject* globalObject,
Expand Down Expand Up @@ -1134,6 +1165,13 @@
RELEASE_AND_RETURN(scope, reject(exception));
}

value = reconcileDataModuleWithRequireCache(globalObject, specifierJS, value);
if (scope.exception()) [[unlikely]] {
auto* exception = scope.exception();
(void)scope.tryClearException();
RELEASE_AND_RETURN(scope, reject(exception));
}

// JSON can become strings, null, numbers, booleans so we must handle "export default 123"
auto function = generateJSValueModuleSourceCode(
globalObject,
Expand All @@ -1151,6 +1189,13 @@
RELEASE_AND_RETURN(scope, reject(JSC::createSyntaxError(globalObject, "Failed to parse Object"_s)));
}

value = reconcileDataModuleWithRequireCache(globalObject, specifierJS, value);
if (scope.exception()) [[unlikely]] {
auto* exception = scope.exception();
(void)scope.tryClearException();
RELEASE_AND_RETURN(scope, reject(exception));
}

// JSON can become strings, null, numbers, booleans so we must handle "export default 123"
auto function = generateJSValueModuleSourceCode(
globalObject,
Expand All @@ -1166,6 +1211,13 @@
RELEASE_AND_RETURN(scope, reject(JSC::createSyntaxError(globalObject, "Failed to parse Object"_s)));
}

value = reconcileDataModuleWithRequireCache(globalObject, specifierJS, value);
if (scope.exception()) [[unlikely]] {
auto* exception = scope.exception();
(void)scope.tryClearException();
RELEASE_AND_RETURN(scope, reject(exception));
}

// JSON can become strings, null, numbers, booleans so we must handle "export default 123"
auto function = generateJSValueExportDefaultObjectSourceCode(
globalObject,
Expand Down
193 changes: 193 additions & 0 deletions test/js/bun/resolve/json-require-import-identity.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,193 @@
import { expect, test } from "bun:test";
import { bunEnv, bunExe, normalizeBunSnapshot, tempDir } from "harness";

// A .json file that is both `import`ed (ESM) and `require`d in the same
// process must yield one shared object. Before this was fixed, the ESM loader
// built a synthetic namespace and a later require() handed that namespace back
// (with a self-referencing `default` key and no identity with the ESM default),
// while require() alone returned the plain data.

async function run(files: Record<string, string>, entry = "index.mjs") {
using dir = tempDir("json-require-import", files);
await using proc = Bun.spawn({
cmd: [bunExe(), entry],
env: bunEnv,
cwd: String(dir),
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
return { stdout: normalizeBunSnapshot(stdout, dir), stderr, exitCode };
}

test.concurrent("require() of a .json already imported via ESM returns the parsed data (no spurious default key)", async () => {
const { stdout, stderr, exitCode } = await run({
"cfg.json": `{"a":1,"b":{"c":[1,2]}}`,
"index.mjs": `
import def from "./cfg.json" with { type: "json" };
import { createRequire } from "node:module";
const req = createRequire(import.meta.url)("./cfg.json");
console.log(JSON.stringify(req));
console.log("same:", req === def);
console.log("own keys:", Object.getOwnPropertyNames(req).sort().join(","));
`,
});
expect(stderr).toBe("");
expect(stdout).toMatchInlineSnapshot(`
"{"a":1,"b":{"c":[1,2]}}
same: true
own keys: a,b"
`);
expect(exitCode).toBe(0);
});

test.concurrent("require() of a .json array already imported via ESM returns the array, not a namespace wrapper", async () => {
const { stdout, stderr, exitCode } = await run({
"arr.json": `[1,2,3]`,
"index.mjs": `
import def from "./arr.json" with { type: "json" };
import { createRequire } from "node:module";
const req = createRequire(import.meta.url)("./arr.json");
console.log(JSON.stringify(req));
console.log("isArray:", Array.isArray(req));
console.log("same:", req === def);
`,
});
expect(stderr).toBe("");
expect(stdout).toMatchInlineSnapshot(`
"[1,2,3]
isArray: true
same: true"
`);
expect(exitCode).toBe(0);
});

test.concurrent("import default of a .json already require()d returns the same object", async () => {
const { stdout, stderr, exitCode } = await run(
{
"cfg.json": `{"a":1}`,
"index.cjs": `
const req = require("./cfg.json");
req.mutated = 42;
(async () => {
const def = (await import("./cfg.json", { with: { type: "json" } })).default;
console.log("same:", req === def);
console.log("mutation:", def.mutated);
console.log(JSON.stringify(def));
})();
`,
},
"index.cjs",
);
expect(stderr).toBe("");
expect(stdout).toMatchInlineSnapshot(`
"same: true
mutation: 42
{"a":1,"mutated":42}"
`);
expect(exitCode).toBe(0);
});

test.concurrent("require.cache[path].exports is the parsed JSON value after an ESM import", async () => {
const { stdout, stderr, exitCode } = await run({
"cfg.json": `{"a":1}`,
"index.mjs": `
import def from "./cfg.json" with { type: "json" };
import { createRequire } from "node:module";
const require = createRequire(import.meta.url);
const key = require.resolve("./cfg.json");
const entry = require.cache[key];
console.log("exports === default:", entry.exports === def);
console.log("loaded:", entry.loaded);
console.log(JSON.stringify(entry.exports));
`,
});
expect(stderr).toBe("");
expect(stdout).toMatchInlineSnapshot(`
"exports === default: true
loaded: true
{"a":1}"
`);
expect(exitCode).toBe(0);
});

test.concurrent("require() and import default of a .toml file share one object", async () => {
const { stdout, stderr, exitCode } = await run({
"cfg.toml": "a = 1\nb = \"hello\"\n",
"index.mjs": `
import def from "./cfg.toml";
import { createRequire } from "node:module";
const req = createRequire(import.meta.url)("./cfg.toml");
console.log(JSON.stringify(req));
console.log("same:", req === def);
`,
});
expect(stderr).toBe("");
expect(stdout).toMatchInlineSnapshot(`
"{"a":1,"b":"hello"}
same: true"
`);
expect(exitCode).toBe(0);
});

test.concurrent("require() of a .json alone (no prior import) still returns the plain data", async () => {
// regression guard: the CJS-only path was already correct.
const { stdout, stderr, exitCode } = await run(
{
"cfg.json": `{"a":1,"b":{"c":[1,2]}}`,
"index.cjs": `
const req = require("./cfg.json");
console.log(JSON.stringify(req));
console.log("own keys:", Object.getOwnPropertyNames(req).sort().join(","));
`,
},
"index.cjs",
);
expect(stderr).toBe("");
expect(stdout).toMatchInlineSnapshot(`
"{"a":1,"b":{"c":[1,2]}}
own keys: a,b"
`);
expect(exitCode).toBe(0);
});

test.concurrent("import * as ns from a .json has no extra synthetic export names", async () => {
// the fix inserts into require.cache; it must not change the ESM namespace shape.
const { stdout, stderr, exitCode } = await run({
"cfg.json": `{"a":1,"b":2}`,
"index.mjs": `
import * as ns from "./cfg.json" with { type: "json" };
console.log(Object.getOwnPropertyNames(ns).sort().join(","));
`,
});
expect(stderr).toBe("");
expect(stdout).toMatchInlineSnapshot(`"a,b,default"`);
expect(exitCode).toBe(0);
});

test.concurrent("delete require.cache[path] after ESM import lets a subsequent require() re-read from disk", async () => {
const { stdout, stderr, exitCode } = await run({
"cfg.json": `{"a":1}`,
"index.mjs": `
import def from "./cfg.json" with { type: "json" };
import { createRequire } from "node:module";
import { writeFileSync } from "node:fs";
const require = createRequire(import.meta.url);
const key = require.resolve("./cfg.json");
const first = require("./cfg.json");
console.log("first === default:", first === def);
delete require.cache[key];
writeFileSync(key, JSON.stringify({ a: 2 }));
const second = require("./cfg.json");
console.log("second.a:", second.a);
console.log("second === first:", second === first);
`,
});
expect(stderr).toBe("");
expect(stdout).toMatchInlineSnapshot(`
"first === default: true
second.a: 2
second === first: false"
`);
expect(exitCode).toBe(0);
});
Loading