Skip to content

fix(schema): only emit referenced $defs in toJsonSchema - #1689

Merged
ssalbdivad merged 1 commit into
arktypeio:mainfrom
giaBaoJS:fix/json-schema-unreferenced-defs
Oct 20, 2026
Merged

ssalbdivad merged 1 commit into
arktypeio:mainfrom
giaBaoJS:fix/json-schema-unreferenced-defs

Conversation

@giaBaoJS

Copy link
Copy Markdown

Fixes #1612, fixes #1511, fixes #1546

All three issues throw the same error from toJsonSchema when refs are in use:

type({ a: "string" }).or("null").toJsonSchema({ useRefs: true })
// Error: Unexpected JSON Schema input for structure: {}

With useRefs, toJsonSchema resolved every root in this.references into $defs. When a union is discriminated, the case nodes it builds are merged into referencesById too. They never show up in the output, and a case can be an intersection with no basis (here { a: string } with the object domain pruned off), so resolving it on its own hits the structure fallthrough and throws. The cyclic issue (#1546) is the same thing, since cyclic types turn useRefs on.

The fix records roots as toJsonSchemaRecurse emits a $ref to them and only resolves those. Iterating the Map also picks up roots first referenced from another definition. One side effect: unreferenced defs no longer show up in the output, which is why union7 drops out of the existing cyclic snapshot (nothing in that schema points to it).

I saw that the featherduster draft (#1685) does the same thing in ark/sets/toJsonSchema.ts. This is the smaller version for the current code, in case it helps before that lands.

Tests: added a useRefs block to ark/type/__tests__/toJsonSchema.test.ts with the repro from each issue. Each test checks that every $ref points to a def and every def is referenced. Without the source change, all three throw the error above. pnpm test passes (1790, up from 1787), and so do pnpm typecheckRepo and lint.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

  • ark/schema/roots/root.ts — toJsonSchema now allocates a ctx.refs Map when refs are in use and resolves only the roots actually reached via $ref, instead of every root in this.references. toJsonSchemaRecurse registers each $ref'd root into that Map, and the live Map iteration picks up roots first reached from another definition.
  • ark/schema/shared/toJsonSchema.ts — adds the optional refs?: Map<string, BaseRoot> field to ToJsonSchema.Context.
  • ark/type/__tests__/toJsonSchema.test.ts — new useRefs block with the repro from each linked issue plus an invariant helper asserting every $ref points at a def and every def is referenced.
  • ark/schema/__tests__/jsonSchema.test.ts — drops the now-unreferenced union7 from the cyclic snapshot.

The fix is sound by construction: $ref is only emitted at root.ts:193, immediately after ctx.refs.set(this.id, this), so every emitted reference is guaranteed a definition, and only referenced roots are resolved. I confirmed the three new tests fail with Unexpected JSON Schema input for structure: {} against the base root.ts, and that pnpm test (1790 passing), pnpm typecheckRepo, and pnpm lint all pass on the head. The union7 drop is correct — nothing in the emitted schema references it.

Pullfrog  | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

With useRefs, every root in node.references was resolved into $defs.
That set also includes discriminated union case nodes, which never
appear in the output and can be intersections without a basis, so
resolving them threw "Unexpected JSON Schema input for structure: {}".

Collect roots as they are emitted as $ref and only resolve those.

Fixes arktypeio#1612, fixes arktypeio#1511, fixes arktypeio#1546
@ssalbdivad
ssalbdivad force-pushed the fix/json-schema-unreferenced-defs branch from 9e0902c to 2503852 Compare October 20, 2026 16:39
@ssalbdivad
ssalbdivad merged commit fc06904 into arktypeio:main Oct 20, 2026
13 checks passed
@github-project-automation github-project-automation Bot moved this from To do to Done (merged or closed) in arktypeio Oct 20, 2026
@ssalbdivad

Copy link
Copy Markdown
Member

thanks @giaBaoJS, love a PR that closes three issues at once! rebased it onto main (restored a flatMorph import #1694 started using) and merged

ssalbdivad added a commit that referenced this pull request Oct 20, 2026
Bumps all publishable packages and documents the changes merged since
2.2.8:

- keep tuple elements around defaults in .in and .out (#1696, @ybai08)
- reject NaN in unions discriminated by typeof (#1702, @thecotne)
- don't treat Object.prototype keys as declared (#1701, @thecotne)
- only emit referenced $defs in toJsonSchema (#1689, fixes #1612,
  #1511 and #1546, @giaBaoJS)
- convert unrepresentable types in ~standard.jsonSchema instead of
  throwing (#1694)
- arkregex: allow } to be escaped (#1691, @cpruijsen)
- attest: onlyNamedTestFiles (#1688)

Also moves attest to TypeScript 7.1's stable native API. Starting with
7.1.0-dev.20261009.1, typescript/unstable/{ast,sync,fs} are exported as
typescript/{ast,sync,fs}, so the published 0.57.0 fails on every current
7.1 build with "Package subpath './unstable/ast' is not defined by
exports". The tsconfig paths aliases for the old specifiers kept
resolving them, so typecheck stayed green while attest crashed at
runtime. They are renamed alongside the imports.

The workspace TypeScript moves to 7.1.0-dev.20261020.3, the latest
nightly (no 7.1 beta is published yet).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
ssalbdivad added a commit that referenced this pull request Oct 20, 2026
Bumps all publishable packages and documents the changes merged since
2.2.8:

- keep tuple elements around defaults in .in and .out (#1696, @ybai08)
- reject NaN in unions discriminated by typeof (#1702, @thecotne)
- don't treat Object.prototype keys as declared (#1701, @thecotne)
- only emit referenced $defs in toJsonSchema (#1689, fixes #1612,
  #1511 and #1546, @giaBaoJS)
- convert unrepresentable types in ~standard.jsonSchema instead of
  throwing (#1694)
- arkregex: allow } to be escaped (#1691, @cpruijsen)
- attest: onlyNamedTestFiles (#1688)

Also moves attest to TypeScript 7.1's native API paths. Starting with
7.1.0-dev.20261009.1, typescript/unstable/{ast,sync,fs} are exported as
typescript/{ast,sync,fs}, so the published 0.57.0 fails on every current
7.1 build with "Package subpath './unstable/ast' is not defined by
exports". The tsconfig paths aliases for the old specifiers kept
resolving them, so typecheck stayed green while attest crashed at
runtime. They are renamed alongside the imports.

The workspace TypeScript moves to 7.1.0-beta, which exports the same
paths.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
ssalbdivad added a commit that referenced this pull request Oct 20, 2026
Bumps all publishable packages and documents the changes merged since
2.2.8:

- keep tuple elements around defaults in .in and .out (#1696, @ybai08)
- reject NaN in unions discriminated by typeof (#1702, @thecotne)
- don't treat Object.prototype keys as declared (#1701, @thecotne)
- only emit referenced $defs in toJsonSchema (#1689, fixes #1612,
  #1511 and #1546, @giaBaoJS)
- convert unrepresentable types in ~standard.jsonSchema instead of
  throwing (#1694)
- arkregex: allow } to be escaped (#1691, @cpruijsen)
- attest: onlyNamedTestFiles (#1688)

Also moves attest to TypeScript 7.1's native API paths. Starting with
7.1.0-dev.20261009.1, typescript/unstable/{ast,sync,fs} are exported as
typescript/{ast,sync,fs}, so the published 0.57.0 fails on every current
7.1 build with "Package subpath './unstable/ast' is not defined by
exports". The tsconfig paths aliases for the old specifiers kept
resolving them, so typecheck stayed green while attest crashed at
runtime. They are renamed alongside the imports.

The workspace TypeScript moves to 7.1.0-beta, which exports the same
paths.
ssalbdivad added a commit that referenced this pull request Oct 21, 2026
Ports the case of fc06904 from arktype 2.2.9 (#1689, @giaBaoJS) that
this branch didn't test. There, toJsonSchema with useRefs resolved every
root in a Type's references into $defs, including a discriminated
union's cases, which can be intersections without a basis, so it threw
"Unexpected JSON Schema input for structure: {}" (#1612, #1511, #1546).

This branch already defines only the roots a $ref reaches, and tests
#1612 and #1546 as "refs to a structure in a union" and "discriminated
union in a recursive scope". #1511's union, whose branches share the
unit "a", is added beside them with the same check: every def is
referenced and every reference defined.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
ssalbdivad added a commit that referenced this pull request Oct 21, 2026
main contributes the 2.2.9 release: version bumps for every publishable
package (arktype 2.2.9, @ark/schema, @ark/util and @ark/fs 0.56.8, attest
0.57.1, arkregex 0.0.14, @ark/json-schema 0.0.13, @ark/fast-check 0.0.20),
attest's move to TypeScript 7.1's native API paths (typescript/{ast,sync,fs}
in place of typescript/unstable/*, with the workspace catalog on
7.1.0-beta), arkdark's regex() highlighting and CI/icon changes, and the
2.2.9 fixes: tuple elements around defaults in .in/.out (#1696), the NaN
check in typeof-discriminated unions (#1702), Object.prototype keys as
undeclared (#1701), only referenced $defs in toJsonSchema (#1689) and
escaped } in arkregex (#1691).

featherduster already had an equivalent of every one of those fixes, each
with main's test or a broader one, so every code conflict keeps
featherduster's side:

- ark/schema constraint.ts, roots/{domain,root,union}.ts,
  structure/{sequence,structure}.ts, shared/toJsonSchema.ts: main patched
  code that featherduster has moved to ark/sets (intersectConstraints,
  discrimination, toJsonSchema) or rewritten. featherduster already narrows
  two constraints with one bound, reduces tuple prefix/optionals around
  defaultables, keeps the NaN check on a typeof case, uses a null-prototype
  propsByKey and resolves only referenced $defs.
- ark/type/__tests__/discrimination.test.ts: main's "number case includes
  NaN" is covered by featherduster's "number case includes NaN and
  Infinity". Its jitless loop is dropped, since testJitless runs the whole
  suite jitless.
- ark/type/__tests__/keywords/number.test.ts: featherduster's messages
  ("must be a number (was NaN)") are kept. A miss of number | null has the
  discriminant code there, so it names only the branch the value reached.
- ark/type/__tests__/match.test.ts: featherduster's caseReference snapshots.
- ark/type/__tests__/toJsonSchema.test.ts: featherduster's referencedDefs
  tests cover main's three useRefs cases (#1546, #1511, #1612).
- CHANGELOGs: featherduster already carried arktype's 2.2.9 and arkregex's
  0.0.14 sections, identical to main's, below its Unreleased sections. In
  attest's, main's 0.57.1 section goes below Unreleased.
- pnpm-lock.yaml: featherduster's, reinstalled against the merged
  catalog, so the only change is typescript 7.1.0-dev.20261006.1 ->
  7.1.0-beta.

The precompiled-module version link moves with the version: runtime.ts
exports precompiledForArktype_2_2_9 in place of precompiledForArktype_2_2_8,
which build.ts's checkVersionExport derives from package.json. The
precompile setup and troubleshooting docs, and llms-full.txt generated
from them, name 2_2_9 and "runs on 2.2.9" in their examples. The docs'
generated util.ts declarations pick up @ark/util 0.56.8.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done (merged or closed)

2 participants