fix(schema): preserve property declaration order from JSON Schema#3016
Open
schani wants to merge 9 commits into
Open
fix(schema): preserve property declaration order from JSON Schema#3016schani wants to merge 9 commits into
schani wants to merge 9 commits into
Conversation
) JSON Schema input always sorted object properties by lowercased name at type-building time in makeObject (JSONSchemaInput.ts), discarding the schema's declaration order before the alphabetizeProperties renderer option ever got a chance to apply. JSON (non-schema) input was unaffected since it doesn't go through this schema-specific sort. Make sortKey optional in makeObject and only sort when a key is given (quicktypePropertyOrder still supplies one when the schema declares it). Otherwise properties keep their original insertion order, matching JSON input, and alphabetizeProperties:true continues to sort at render time. Adds a schema fixture (property-order.schema/.1.json) covering both default declaration order and quicktypePropertyOrder, verified via a generated-vs-source key-order check in the typescript fixture driver. Co-Authored-By: gpt-5.6-sol via pi <noreply@openai.com>
Co-Authored-By: Claude <noreply@anthropic.com>
…sing required properties (#undefined) cjson silently defaults a missing required field to 0 instead of failing, the same known limitation already documented for required.schema and intersection.schema. This matched CI's property-order.1.fail.no-defaults.json exiting 0 instead of failing as expected. Co-Authored-By: gpt-5.6-sol via pi <noreply@openai.com>
Brings the PR branch up to date with master (which the pull_request CI check merges against). Also fixes the one regression the merge surfaces: test/unit/cjson-enum-default.test.ts (added on master after this branch diverged) hardcoded alphabetical cJSON enum member order. This branch's fix (issue #2698) intentionally preserves JSON Schema declaration order instead, so the generated enum for the test's schema now correctly emits members as state, config, heartbeat (declaration order) rather than config, heartbeat, state (alphabetical) - the sentinel-avoidance numbering (starting at 1) is unchanged. Updated the test's expected member order to match. Co-Authored-By: gpt-5.6-sol via pi <noreply@openai.com>
…ot detect missing required properties (#undefined) Same root cause as the earlier cjson fix on this branch: both languages already skip required.schema/intersection.schema for the identical documented reason (Elixir sets absent struct keys to null at runtime; Haskell's fixture driver serializes a failed Maybe decode as JSON null and exits 0), so property-order.1.fail.no-defaults.json (omits required "banana", no default) cannot be detected as a failure by either. Co-Authored-By: gpt-5.6-sol via pi <noreply@openai.com>
Resolves additive conflict in test/languages.ts Elixir skipSchema list: keep both property-order.schema (this branch) and optional-any.schema (master). Co-Authored-By: Claude <noreply@anthropic.com>
Generated-output differences840 files differ — 809 modified, 31 new, 0 deleted |
Generated-output differences871 files differ — 839 modified, 32 new, 0 deleted |
schani
commented
Jul 23, 2026
| const value = TopLevel.Convert.toTopLevel(json); | ||
| const backToJson = TopLevel.Convert.topLevelToJson(value); | ||
|
|
||
| if (sample.endsWith("property-order.1.json")) { |
Member
Author
There was a problem hiding this comment.
This is a shit way of testing this. The fixtures should never special-case for specific test cases!!! Find a clean way of testing this!
# Conflicts: # test/languages.ts
Generated-output differences872 files differ — 840 modified, 32 new, 0 deleted |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
The bug
alphabetizeProperties: false(the default) had no effect on JSON Schemainput — generated properties were always alphabetized, regardless of the
option. The equivalent JSON (non-schema) input correctly preserved
declaration order.
Repro:
with a schema declaring properties
zebra, mango, apple, delta, bananaproduced
apple, banana, delta, mango, zebra(alphabetized) instead ofthe schema's declaration order.
Root cause
makeObjectinpackages/quicktype-core/src/input/JSONSchemaInput.tsdefaulted its
sortKeyparameter to(k) => k.toLowerCase()and alwaysran properties through
mapSortBy, discarding the schema's declaredproperty order at type-building time — before the renderer-level
alphabetizePropertiesoption (inConvenienceRenderer.ts) ever got achance to apply. That renderer option only ever adds alphabetical
sorting when explicitly set to
true; it has no way to recover an orderthat was already thrown away upstream. JSON (non-schema) input never went
through this code path, so it worked correctly.
The fix
sortKeyis now optional. When no sort key is supplied (the common case),properties keep their original object insertion order (which reflects
schema declaration order). The one place that already computed a sort key
— the
quicktypePropertyOrderschema extension — continues to workexactly as before.
alphabetizeProperties: truecontinues to sort atrender time, unaffected by this change.
Test coverage
Added
test/inputs/schema/property-order.schemawith a correspondingproperty-order.1.jsonsample:default declaration-order preservation
orderedobject using thequicktypePropertyOrderextension,to verify that mechanism still works
The
typescriptfixture driver (test/fixtures/typescript/main.ts) wasextended to assert, for this specific sample, that the generated code's
round-tripped JSON key order matches the input's key order — this is the
only fixture-level way to observe property order rather than just
round-trip value correctness.
Verification
pre-fix code (alphabetized order didn't match expected declaration
order).
preserves declaration order (
zebra, mango, apple, delta, banana).--alphabetize-propertiesstill alphabetizes asexpected.
npm run buildpasses.QUICKTEST=true FIXTURE=schema-typescript script/test— all 71 testspass, including the new
property-order.schemafixture.cover the remaining languages.
Fixes #2698.
🤖 Generated with Claude Code