Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
57 changes: 51 additions & 6 deletions src/js_printer/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5244,18 +5244,22 @@ pub mod __gated_printer {
self.print_whitespacer(ws!(b"from "));
}

let irp = &self.import_record(s.import_record_index as usize).path.text;
self.print_import_record_path(
self.import_record(s.import_record_index as usize),
);
let record = self.import_record(s.import_record_index as usize);
let irp = &record.path.text;
let implies_json = Self::record_implies_json_type(record);
self.print_import_record_path(record);
if IS_BUN_PLATFORM && implies_json {
self.print_whitespacer(ws!(b" with { type: \"json\" }"));
}
self.print_semicolon_after_statement();

if Self::MAY_HAVE_MODULE_INFO {
if let Some(mi) = self.module_info() {
let irp_id = mi.str(irp);
use analyze_transpiled_module::FetchParameters as FP;
mi.request_module(
irp_id,
analyze_transpiled_module::FetchParameters::None,
if implies_json { FP::Json } else { FP::None },
);
if let Some(alias) = &s.alias {
let alias_id = mi.str(alias.original_name.slice());
Expand Down Expand Up @@ -5481,7 +5485,11 @@ pub mod __gated_printer {

self.print_whitespacer(ws!(b"} from "));
let irp = &import_record.path.text;
let implies_json = Self::record_implies_json_type(import_record);
self.print_import_record_path(import_record);
if IS_BUN_PLATFORM && implies_json {
self.print_whitespacer(ws!(b" with { type: \"json\" }"));
}
self.print_semicolon_after_statement();

if Self::MAY_HAVE_MODULE_INFO && self.module_info.is_some() {
Expand All @@ -5490,7 +5498,8 @@ pub mod __gated_printer {
let irp_id = {
let mi = self.module_info().expect("infallible: module_info enabled");
let id = mi.str(irp);
mi.request_module(id, analyze_transpiled_module::FetchParameters::None);
use analyze_transpiled_module::FetchParameters as FP;
mi.request_module(id, if implies_json { FP::Json } else { FP::None });
id
};
for item in slice_of(s.items).iter() {
Expand Down Expand Up @@ -5999,6 +6008,8 @@ pub mod __gated_printer {
self.print_whitespacer(ws!(b" with { type: \"md\" }"))
}
}
} else if Self::record_implies_json_type(record) {
self.print_whitespacer(ws!(b" with { type: \"json\" }"));
Comment thread
robobun marked this conversation as resolved.
}
}
self.print_semicolon_after_statement();
Expand Down Expand Up @@ -6039,6 +6050,8 @@ pub mod __gated_printer {
Loader::Json5 => FP::host_defined(mi.str(b"json5")),
Loader::Md => FP::host_defined(mi.str(b"md")),
}
} else if Self::record_implies_json_type(record) {
FP::Json
} else {
FP::None
}
Expand Down Expand Up @@ -6179,6 +6192,38 @@ pub mod __gated_printer {
self.print(b"module.exports");
}

/// True when `record` carries no `with { type }` attribute but its
/// specifier would be loaded with Bun's JSON loader. For such records
/// the Bun-target printer emits an explicit `with { type: "json" }`
/// clause so JSC's module map (keyed on
/// `(specifier, ScriptFetchParameters::Type)`) hashes the attribute-less
/// and attributed forms to the same slot; without it the former keys on
/// `Type::JavaScript`, the latter on `Type::JSON`, and one file becomes
/// two live module instances.
///
/// Filenames Bun routes to the `jsonc` loader instead of `json`
/// (`loader_for_path` in `jsc_hooks.rs`) are excluded: emitting
/// `with { type: "json" }` for those would force strict JSON at fetch
/// time and break empty/commented `package.json` / `tsconfig.json`.
Comment thread
robobun marked this conversation as resolved.
Outdated
fn record_implies_json_type(record: &ImportRecord) -> bool {
if record.loader.is_some() {
return false;
}
let text = record.path.text;
let end = text
.iter()
.position(|&c| c == b'?' || c == b'#')
.unwrap_or(text.len());
let path = &text[..end];
Comment thread
robobun marked this conversation as resolved.
Outdated
if !strings::has_suffix_comptime(path, b".json") {
return false;
}
let filename = bun_paths::basename(path);
!(filename == b"package.json"
|| strings::has_prefix_comptime(filename, b"tsconfig.")
|| strings::has_prefix_comptime(filename, b"jsconfig."))
}
Comment thread
claude[bot] marked this conversation as resolved.

pub fn print_import_record_path(&mut self, import_record: &ImportRecord) {
if IS_JSON {
unreachable!();
Expand Down
31 changes: 31 additions & 0 deletions src/jsc/bindings/ZigGlobalObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3650,6 +3650,35 @@ JSC::Identifier GlobalObject::moduleLoaderResolve(JSGlobalObject* jsGlobalObject
}
}

// Bun loads a `.json` path with the JSON loader whether or not the request
// carried `with { type: "json" }`. JSC's module map is keyed on
// (specifier, ScriptFetchParameters::Type), so an attribute-less request must
// reach it as Type::JSON too or the two forms become two live module
// instances. Applied only when the caller supplied no attributes (an explicit
// `with { type: ... }` is left untouched) and the filename is not one Bun
// routes to the jsonc loader (`loader_for_path` in jsc_hooks.rs), for which an
// inferred `type: "json"` would force strict JSON at fetch time.
Comment thread
robobun marked this conversation as resolved.
Outdated
static ALWAYS_INLINE void normalizeFetchParametersForResolvedPath(RefPtr<JSC::ScriptFetchParameters>& parameters, const JSC::Identifier& resolved)
{
if (parameters)
return;
auto* impl = resolved.impl();
if (!impl || impl->isSymbol())
return;
StringView path(*impl);
if (size_t q = path.find([](char16_t c) { return c == '?' || c == '#'; }); q != notFound)
path = path.left(q);
if (!path.endsWith(".json"_s))
return;
size_t slash = path.reverseFind('/');
size_t backslash = path.reverseFind('\\');
size_t sep = slash == notFound ? backslash : (backslash == notFound ? slash : std::max(slash, backslash));
StringView filename = sep == notFound ? path : path.substring(sep + 1);
if (filename == "package.json"_s || filename.startsWith("tsconfig."_s) || filename.startsWith("jsconfig."_s))
return;
parameters = JSC::ScriptFetchParameters::create(JSC::ScriptFetchParameters::Type::JSON);
}

JSC::JSPromise* GlobalObject::moduleLoaderImportModule(JSGlobalObject* jsGlobalObject,
JSModuleLoader*,
JSString* moduleNameValue,
Expand Down Expand Up @@ -3698,6 +3727,7 @@ JSC::JSPromise* GlobalObject::moduleLoaderImportModule(JSGlobalObject* jsGlobalO
if (globalObject->onLoadPlugins.hasVirtualModules()) {
if (auto resolution = globalObject->onLoadPlugins.resolveVirtualModule(moduleName, sourceURL.protocolIsFile() ? sourceOriginStringHolder : String())) {
resolvedIdentifier = JSC::Identifier::fromString(vm, resolution.value());
normalizeFetchParametersForResolvedPath(parameters, resolvedIdentifier);

auto result = JSC::importModule(globalObject, resolvedIdentifier, JSC::Identifier(), parameters, nullptr, /* deferred */ false, referrerAsyncOrder);
if (scope.exception()) [[unlikely]] {
Expand Down Expand Up @@ -3758,6 +3788,7 @@ JSC::JSPromise* GlobalObject::moduleLoaderImportModule(JSGlobalObject* jsGlobalO
// The C++ module loader now extracts `with.type` into a
// ScriptFetchParameters before calling this hook, so `parameters` is
// already the parsed RefPtr (or null). Just forward it.
normalizeFetchParametersForResolvedPath(parameters, resolvedIdentifier);
auto result = JSC::importModule(globalObject, resolvedIdentifier,
JSC::Identifier(), WTF::move(parameters), nullptr, /* deferred */ false, referrerAsyncOrder);
if (scope.exception()) [[unlikely]] {
Expand Down
191 changes: 191 additions & 0 deletions test/js/bun/resolve/json-import-identity.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,191 @@
import { describe, expect, test } from "bun:test";
import { bunEnv, bunExe, normalizeBunSnapshot, tempDir } from "harness";

// Bun accepts `import "./x.json"` without `with { type: "json" }`. Both forms
// load the same file with the same loader, so they must resolve to the same
// module record in JSC's registry. Before this was fixed the attribute-less
// form keyed on ScriptFetchParameters::Type::JavaScript and the attributed
// form on Type::JSON, so two module instances were created and a mutation via
// one was invisible via the other.

async function run(files: Record<string, string>) {
using dir = tempDir("json-import-identity", files);
await using proc = Bun.spawn({
cmd: [bunExe(), "index.mjs"],
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("static .json imports with and without the type attribute share one module across files", async () => {

Check warning on line 28 in test/js/bun/resolve/json-import-identity.test.ts

View check run for this annotation

Claude / Claude Code Review

14 independent subprocess tests should use test.concurrent

nit: All 14 tests here spawn independent subprocesses via `run()` (each with its own `tempDir`, no shared state), so they should use `test.concurrent` — 14 serial debug+ASAN spawns will push this file well past the ~10s budget. See sibling `test/js/bun/resolve/bun-main-entry-point.test.ts` for the same pattern.
Comment thread
robobun marked this conversation as resolved.
Outdated
const { stdout, exitCode } = await run({
"cfg.json": `{"n":1}`,
"plain.mjs": `import a from "./cfg.json"; export default a;`,
"attr.mjs": `import b from "./cfg.json" with { type: "json" }; export default b;`,
"index.mjs": `
const plain = (await import("./plain.mjs")).default;
const attr = (await import("./attr.mjs")).default;
console.log("same:", plain === attr);
plain.n = 42;
console.log("mutation:", attr.n);
`,
});
expect(stdout).toMatchInlineSnapshot(`
"same: true
mutation: 42"
`);
expect(exitCode).toBe(0);
Comment thread
robobun marked this conversation as resolved.
});

test("static .json imports share one module regardless of load order", async () => {
const { stdout, exitCode } = await run({
"cfg.json": `{"n":1}`,
"plain.mjs": `import a from "./cfg.json"; export default a;`,
"attr.mjs": `import b from "./cfg.json" with { type: "json" }; export default b;`,
"index.mjs": `
const attr = (await import("./attr.mjs")).default;
const plain = (await import("./plain.mjs")).default;
console.log("same:", plain === attr);
`,
});
expect(stdout).toMatchInlineSnapshot(`"same: true"`);
expect(exitCode).toBe(0);
});

test("dynamic import() of a .json with and without the type attribute returns one module", async () => {
const { stdout, exitCode } = await run({
"cfg.json": `{"n":1}`,
"index.mjs": `
const plain = await import("./cfg.json");
const attr = await import("./cfg.json", { with: { type: "json" } });
console.log("ns:", plain === attr);
console.log("default:", plain.default === attr.default);
plain.default.n = 99;
console.log("mutation:", attr.default.n);
`,
});
expect(stdout).toMatchInlineSnapshot(`
"ns: true
default: true
mutation: 99"
`);
expect(exitCode).toBe(0);
});

test("dynamic import() of a .json shares one module regardless of order", async () => {
const { stdout, exitCode } = await run({
"cfg.json": `{"n":1}`,
"index.mjs": `
const attr = await import("./cfg.json", { with: { type: "json" } });
const plain = await import("./cfg.json");
console.log("ns:", plain === attr);
`,
});
expect(stdout).toMatchInlineSnapshot(`"ns: true"`);
expect(exitCode).toBe(0);
});

test("a static attribute-less .json import and a dynamic attributed one share one module", async () => {
const { stdout, exitCode } = await run({
"cfg.json": `{"n":1}`,
"plain.mjs": `import a from "./cfg.json"; export default a;`,
"index.mjs": `
const plain = (await import("./plain.mjs")).default;
const attr = (await import("./cfg.json", { with: { type: "json" } })).default;
console.log("same:", plain === attr);
`,
});
expect(stdout).toMatchInlineSnapshot(`"same: true"`);
expect(exitCode).toBe(0);
});

test("export-from of a .json shares one module with an attributed import", async () => {
const { stdout, exitCode } = await run({
"cfg.json": `{"n":1}`,
"reex.mjs": `export { default as cfg } from "./cfg.json";`,
"imp.mjs": `import b from "./cfg.json" with { type: "json" }; export default b;`,
"index.mjs": `
const a = (await import("./reex.mjs")).cfg;
const b = (await import("./imp.mjs")).default;
console.log("same:", a === b);
`,
});
expect(stdout).toMatchInlineSnapshot(`"same: true"`);
expect(exitCode).toBe(0);
});

test("an explicit non-json type attribute still produces a distinct module", async () => {
const { stdout, exitCode } = await run({
"cfg.json": `{"n":1}`,
"index.mjs": `
const asJson = (await import("./cfg.json", { with: { type: "json" } })).default;
const asText = (await import("./cfg.json", { with: { type: "text" } })).default;
console.log("json:", JSON.stringify(asJson));
console.log("text:", asText);
console.log("distinct:", asJson !== asText);
`,
});
expect(stdout).toMatchInlineSnapshot(`
"json: {"n":1}
text: {"n":1}
distinct: true"
`);
expect(exitCode).toBe(0);
});

test("a .json specifier with a query string still normalizes to one module", async () => {
const { stdout, exitCode } = await run({
"cfg.json": `{"n":1}`,
"index.mjs": `
const plain = await import("./cfg.json?v=1");
const attr = await import("./cfg.json?v=1", { with: { type: "json" } });
console.log("same:", plain.default === attr.default);
`,
});
expect(stdout).toMatchInlineSnapshot(`"same: true"`);
expect(exitCode).toBe(0);
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

describe("jsonc-loaded filenames are left alone", () => {
// package.json / tsconfig.json / jsconfig.json use Bun's jsonc loader even
// though the extension is `.json`. The normalization must not synthesize
// `with { type: "json" }` for them: that would reach the fetch hook as an
// explicit `type` override and force strict JSON, breaking Bun's lenient
// handling of empty / commented config files.
for (const name of ["package.json", "tsconfig.json", "jsconfig.json"]) {
test(`static import of an empty ${name} still works`, async () => {
const { stdout, stderr, exitCode } = await run({
[name]: ``,
"plain.mjs": `import a from "./${name}"; export default a;`,
"index.mjs": `
const a = (await import("./plain.mjs")).default;
console.log(JSON.stringify(a));
`,
});
expect(stderr).not.toContain("JSON Parse error");
expect(stdout).toMatchInlineSnapshot(`"{}"`);
expect(exitCode).toBe(0);
});

test(`dynamic import of an empty ${name} still works`, async () => {
const { stdout, stderr, exitCode } = await run({
[name]: ``,
"index.mjs": `
const a = (await import("./${name}")).default;
console.log(JSON.stringify(a));
`,
});
expect(stderr).not.toContain("JSON Parse error");
expect(stdout).toMatchInlineSnapshot(`"{}"`);
expect(exitCode).toBe(0);
});
}
});
Loading