-
Notifications
You must be signed in to change notification settings - Fork 5k
runtime: one module record for a .json imported with and without the type attribute #35914
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
robobun
wants to merge
6
commits into
main
Choose a base branch
from
farm/edab56f9/json-import-attr-identity
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+396
−7
Open
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
a0bcfa1
runtime: one module record for a .json imported with and without the …
robobun 0ae9cca
[autofix.ci] apply automated fixes
autofix-ci[bot] 746dc52
tighten doc comments; add export-star and transpiler-output coverage
robobun bfbc4cf
normalize on the as-written specifier on both sides; skip ?raw; use t…
robobun ce0b666
bump transpiler cache version; split specifiers on `?` only
robobun 280ee2d
test: assert empty stderr inside run()
robobun File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,231 @@ | ||
| 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 () => { | ||
|
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); | ||
|
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("export * as of a .json shares one module with an attributed import", async () => { | ||
| const { stdout, exitCode } = await run({ | ||
| "cfg.json": `{"n":1}`, | ||
| "reex.mjs": `export * 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.default === b); | ||
| `, | ||
| }); | ||
| expect(stdout).toMatchInlineSnapshot(`"same: true"`); | ||
| expect(exitCode).toBe(0); | ||
| }); | ||
|
|
||
| test('the bun-target transpiler emits `with { type: "json" }` for attribute-less .json specifiers', () => { | ||
| // Static child imports go through `hostLoadImportedModule`, not the | ||
| // dynamic-import hook, so the printer change is load bearing on its own. | ||
| const t = new Bun.Transpiler({ target: "bun" }); | ||
| const out = t.transformSync( | ||
| [ | ||
| `import a from "./cfg.json";`, | ||
| `import b from "./package.json";`, | ||
| `import c from "./data.json" with { type: "text" };`, | ||
| `export { default as d } from "./other.json";`, | ||
| `export * as e from "./more.json";`, | ||
| ].join("\n"), | ||
| ); | ||
| expect(out).toMatchInlineSnapshot(` | ||
| "import a from "./cfg.json" with { type: "json" }; | ||
| import b from "./package.json"; | ||
| import c from "./data.json" with { type: "text" }; | ||
| export { default as d } from "./other.json" with { type: "json" }; | ||
| export * as e from "./more.json" with { type: "json" }; | ||
| " | ||
| `); | ||
| // Other targets are untouched. | ||
| for (const target of ["browser", "node"] as const) { | ||
| expect(new Bun.Transpiler({ target }).transformSync(`import a from "./cfg.json";`)).toBe( | ||
| `import a from "./cfg.json";\n`, | ||
| ); | ||
| } | ||
| }); | ||
|
|
||
| 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); | ||
| }); | ||
|
|
||
| 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); | ||
| }); | ||
| } | ||
| }); | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.