From 732ef8c5e72f4581b14db356118ef17ffae60b4f Mon Sep 17 00:00:00 2001 From: Mark Probst Date: Mon, 20 Jul 2026 17:26:31 -0400 Subject: [PATCH 1/4] fix(js): export all-objects converters from module.exports (#1655) With --converters all-objects, the JavaScript renderer generates toX/xToJson functions for every object type, but module.exports only listed the top-level converters, making the extra converters unreachable by callers of the module. Co-Authored-By: gpt-5.6-sol via pi --- .../src/language/JavaScript/JavaScriptRenderer.ts | 14 ++++++++++++-- test/fixtures/javascript/main.js | 6 ++++++ test/inputs/json/samples/issue-1655.json | 5 +++++ test/languages.ts | 1 + 4 files changed, 24 insertions(+), 2 deletions(-) create mode 100644 test/inputs/json/samples/issue-1655.json diff --git a/packages/quicktype-core/src/language/JavaScript/JavaScriptRenderer.ts b/packages/quicktype-core/src/language/JavaScript/JavaScriptRenderer.ts index c14a44efdb..3d707b69a8 100644 --- a/packages/quicktype-core/src/language/JavaScript/JavaScriptRenderer.ts +++ b/packages/quicktype-core/src/language/JavaScript/JavaScriptRenderer.ts @@ -554,12 +554,22 @@ function r(name${stringAnnotation}) { this.ensureBlankLine(); this.emitBlock("module.exports = ", ";", () => { - this.forEachTopLevel("none", (_, name) => { + const exporter = (_: Type, name: Name): void => { const serializer = this.serializerFunctionName(name); const deserializer = this.deserializerFunctionName(name); this.emitLine('"', serializer, '": ', serializer, ","); this.emitLine('"', deserializer, '": ', deserializer, ","); - }); + }; + + switch (this._jsOptions.converters) { + case ConvertersOptions.AllObjects: + this.forEachObject("none", exporter); + break; + + default: + this.forEachTopLevel("none", exporter); + break; + } }); } diff --git a/test/fixtures/javascript/main.js b/test/fixtures/javascript/main.js index cb60dd70d8..e52fe6e2a9 100644 --- a/test/fixtures/javascript/main.js +++ b/test/fixtures/javascript/main.js @@ -9,4 +9,10 @@ const json = fs.readFileSync(sample); const value = TopLevel.toTopLevel(json); const backToJson = TopLevel.topLevelToJson(value); +const generatedSource = fs.readFileSync(require.resolve("./TopLevel"), "utf8"); +if (generatedSource.includes("function toData123(")) { + const data123 = TopLevel.toData123(JSON.stringify(value.data123)); + TopLevel.data123ToJson(data123); +} + console.log(backToJson); diff --git a/test/inputs/json/samples/issue-1655.json b/test/inputs/json/samples/issue-1655.json new file mode 100644 index 0000000000..3f32cc9bad --- /dev/null +++ b/test/inputs/json/samples/issue-1655.json @@ -0,0 +1,5 @@ +{ + "data123": { + "name": "quicktype" + } +} diff --git a/test/languages.ts b/test/languages.ts index 6bcfd43611..6f2772924c 100644 --- a/test/languages.ts +++ b/test/languages.ts @@ -1024,6 +1024,7 @@ export const JavaScriptLanguage: Language = { { "runtime-typecheck": "false" }, { "runtime-typecheck-ignore-unknown-properties": "true" }, { converters: "top-level" }, + ["issue-1655.json", { converters: "all-objects" }], ], sourceFiles: ["src/language/JavaScript/index.ts"], }; From 37d8d9227c754ee810fa7fe3e0f8a0cbb08d153a Mon Sep 17 00:00:00 2001 From: Mark Probst Date: Tue, 21 Jul 2026 11:35:58 -0400 Subject: [PATCH 2/4] test(js): verify all defined converters are exported (#1655) The issue-1655 fixture driver decided whether to exercise the nested data123 converter by matching the generated source for the exact string "function toData123(". Replace that brittle hard-coded gate with a check of the invariant #1655 actually establishes: every converter the generated module defines (`function to(json)` / `function ToJson(value)`) must also be reachable through module.exports. The module's helper functions have different signatures and are not matched, so the check is general across samples and converter modes rather than tied to one sample name. The same sample is rendered with several converter options and the driver only receives the sample path, so the set of emitted converters is read from the generated module; in the default (top-level) rendering data123 is simply never defined and never checked. Co-Authored-By: Claude --- test/fixtures/javascript/main.js | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/test/fixtures/javascript/main.js b/test/fixtures/javascript/main.js index e52fe6e2a9..b815ed6732 100644 --- a/test/fixtures/javascript/main.js +++ b/test/fixtures/javascript/main.js @@ -9,10 +9,27 @@ const json = fs.readFileSync(sample); const value = TopLevel.toTopLevel(json); const backToJson = TopLevel.topLevelToJson(value); +// Regression check for #1655: every converter the generated module defines +// must also be listed in module.exports, or callers of the module can't reach +// it. With `--converters all-objects` this includes a `to`/`ToJson` +// pair per object type (e.g. the nested `data123` object), which used to be +// generated but never exported. The same sample is rendered with several +// converter options, and the driver only receives the sample path, so we +// verify the invariant against whichever converters the current options +// actually emitted rather than assuming a fixed set. Converters have the +// distinctive signatures `function to(json)` and +// `function ToJson(value)`; the module's helper functions do not. const generatedSource = fs.readFileSync(require.resolve("./TopLevel"), "utf8"); -if (generatedSource.includes("function toData123(")) { - const data123 = TopLevel.toData123(JSON.stringify(value.data123)); - TopLevel.data123ToJson(data123); +const definedConverters = [ + ...generatedSource.matchAll(/^function (to[A-Z]\w*)\(json\)/gm), + ...generatedSource.matchAll(/^function (\w+ToJson)\(value\)/gm), +].map((match) => match[1]); +for (const name of definedConverters) { + if (typeof TopLevel[name] !== "function") { + throw new Error( + `converter ${name} is defined in the generated module but is not exported from module.exports`, + ); + } } console.log(backToJson); From de6e023216b0476d45abda3a2e663606f111b0ac Mon Sep 17 00:00:00 2001 From: Mark Probst Date: Tue, 21 Jul 2026 12:08:38 -0400 Subject: [PATCH 3/4] test(js): assert all-objects export invariant in a unit test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fixture driver read the generated TopLevel.js back and grepped its source for converter definitions to check they were all exported. That inspection of emitted source belongs in a render-and-inspect unit test, not in the round-trip fixture driver. Move the #1655 regression check into test/unit: render the JS module with converters: all-objects and assert every defined converter appears in the module.exports block (plus a top-level control that shows the nested converter is neither generated nor exported in the default mode). This mirrors the existing java-acronym-names unit test, whose comment explains the same rationale — a round-trip fixture cannot observe the defect because the top-level converter it exercises works regardless. Reverting the renderer fix makes the new test fail. The javascript fixture driver returns to a plain round-trip; issue-1655.json still renders with converters: all-objects, keeping fixture coverage that the mode generates runnable code. Co-Authored-By: Claude --- test/fixtures/javascript/main.js | 23 ----- .../javascript-all-objects-exports.test.ts | 85 +++++++++++++++++++ 2 files changed, 85 insertions(+), 23 deletions(-) create mode 100644 test/unit/javascript-all-objects-exports.test.ts diff --git a/test/fixtures/javascript/main.js b/test/fixtures/javascript/main.js index b815ed6732..cb60dd70d8 100644 --- a/test/fixtures/javascript/main.js +++ b/test/fixtures/javascript/main.js @@ -9,27 +9,4 @@ const json = fs.readFileSync(sample); const value = TopLevel.toTopLevel(json); const backToJson = TopLevel.topLevelToJson(value); -// Regression check for #1655: every converter the generated module defines -// must also be listed in module.exports, or callers of the module can't reach -// it. With `--converters all-objects` this includes a `to`/`ToJson` -// pair per object type (e.g. the nested `data123` object), which used to be -// generated but never exported. The same sample is rendered with several -// converter options, and the driver only receives the sample path, so we -// verify the invariant against whichever converters the current options -// actually emitted rather than assuming a fixed set. Converters have the -// distinctive signatures `function to(json)` and -// `function ToJson(value)`; the module's helper functions do not. -const generatedSource = fs.readFileSync(require.resolve("./TopLevel"), "utf8"); -const definedConverters = [ - ...generatedSource.matchAll(/^function (to[A-Z]\w*)\(json\)/gm), - ...generatedSource.matchAll(/^function (\w+ToJson)\(value\)/gm), -].map((match) => match[1]); -for (const name of definedConverters) { - if (typeof TopLevel[name] !== "function") { - throw new Error( - `converter ${name} is defined in the generated module but is not exported from module.exports`, - ); - } -} - console.log(backToJson); diff --git a/test/unit/javascript-all-objects-exports.test.ts b/test/unit/javascript-all-objects-exports.test.ts new file mode 100644 index 0000000000..7950c6b886 --- /dev/null +++ b/test/unit/javascript-all-objects-exports.test.ts @@ -0,0 +1,85 @@ +// Regression test for #1655. With `--converters all-objects` the JavaScript +// renderer emits a `to`/`ToJson` converter pair for every object +// type, not just the top-level ones. Before the fix, `emitModuleExports()` +// still iterated only the top-level types, so those extra per-object +// converters were generated in the module body but never listed in +// `module.exports` — callers of the module could not reach them. +// +// A round-trip fixture cannot catch this: the driver deserializes and +// reserializes through the top-level converter, which is exported and works +// regardless of whether the nested converters are. This test therefore +// generates the module directly and inspects its `module.exports` block. + +import { + InputData, + jsonInputForTargetLanguage, + quicktype, +} from "quicktype-core"; +import { describe, expect, test } from "vitest"; + +// A schema with a nested object (`data123`) so the all-objects renderer emits a +// converter for a non-top-level type. +const sample = JSON.stringify({ data123: { name: "quicktype" } }); + +async function renderJavaScript(converters: string): Promise { + const jsonInput = jsonInputForTargetLanguage("js"); + await jsonInput.addSource({ name: "TopLevel", samples: [sample] }); + const inputData = new InputData(); + inputData.addInput(jsonInput); + + const result = await quicktype({ + inputData, + lang: "js", + rendererOptions: { "acronym-style": "pascal", converters }, + }); + return result.lines.join("\n"); +} + +// The `module.exports = { ... };` object literal at the bottom of the module. +function moduleExportsBlock(source: string): string { + const match = source.match(/module\.exports\s*=\s*\{([\s\S]*?)\};/); + // biome-ignore lint/suspicious/noMisplacedAssertion: helper is only called from within tests + expect(match, `no module.exports block in:\n${source}`).not.toBeNull(); + return match?.[1] ?? ""; +} + +// Every converter the module defines — `function to(json)` and +// `function ToJson(value)`. The module's helpers (cast, uncast, +// transform, …) have different signatures and are intentionally not matched. +function definedConverters(source: string): string[] { + return [ + ...source.matchAll(/^function (to[A-Z]\w*)\(json\)/gm), + ...source.matchAll(/^function (\w+ToJson)\(value\)/gm), + ].map((m) => m[1]); +} + +describe("JavaScript converters: all-objects module.exports", () => { + test("exports a converter for every object type, including nested ones", async () => { + const source = await renderJavaScript("all-objects"); + const converters = definedConverters(source); + const exportsBlock = moduleExportsBlock(source); + + // Sanity: the renderer actually emitted a nested-object converter. + expect(converters).toContain("toData123"); + expect(converters).toContain("data123ToJson"); + + // The invariant #1655 restores: every defined converter is exported. + for (const name of converters) { + expect( + exportsBlock, + `converter ${name} is defined but not in module.exports`, + ).toContain(name); + } + }); + + test("top-level mode exports only top-level converters", async () => { + const source = await renderJavaScript("top-level"); + const exportsBlock = moduleExportsBlock(source); + + expect(exportsBlock).toContain("toTopLevel"); + // The nested converter is not generated in top-level mode, so it must + // not appear in the exports either — this is what distinguishes the + // all-objects behavior above from the default. + expect(exportsBlock).not.toContain("toData123"); + }); +}); From 767587e5702bbafd0382014ad2173f997f58c31b Mon Sep 17 00:00:00 2001 From: Mark Probst Date: Tue, 21 Jul 2026 20:42:02 -0400 Subject: [PATCH 4/4] test(js): reuse nested objects fixture for all-objects --- test/inputs/json/samples/issue-1655.json | 5 ----- test/languages.ts | 2 +- 2 files changed, 1 insertion(+), 6 deletions(-) delete mode 100644 test/inputs/json/samples/issue-1655.json diff --git a/test/inputs/json/samples/issue-1655.json b/test/inputs/json/samples/issue-1655.json deleted file mode 100644 index 3f32cc9bad..0000000000 --- a/test/inputs/json/samples/issue-1655.json +++ /dev/null @@ -1,5 +0,0 @@ -{ - "data123": { - "name": "quicktype" - } -} diff --git a/test/languages.ts b/test/languages.ts index 6f2772924c..9eb590760e 100644 --- a/test/languages.ts +++ b/test/languages.ts @@ -1024,7 +1024,7 @@ export const JavaScriptLanguage: Language = { { "runtime-typecheck": "false" }, { "runtime-typecheck-ignore-unknown-properties": "true" }, { converters: "top-level" }, - ["issue-1655.json", { converters: "all-objects" }], + ["nested-objects.json", { converters: "all-objects" }], ], sourceFiles: ["src/language/JavaScript/index.ts"], };