-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Improve metadata tracking across child-parent relationships #5578
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -75,7 +75,10 @@ export interface Seen { | |||||||||||||||||
| /** Cycle path */ | ||||||||||||||||||
| cycle?: (string | number)[] | undefined; | ||||||||||||||||||
| isParent?: boolean | undefined; | ||||||||||||||||||
| ref?: schemas.$ZodType | undefined | null; | ||||||||||||||||||
| /** Schema to inherit JSON Schema properties from (set by processor for wrappers) */ | ||||||||||||||||||
| ref?: schemas.$ZodType | null; | ||||||||||||||||||
| /** Parent schema in the clone chain (for $ref propagation when parent is extracted) */ | ||||||||||||||||||
| parent?: schemas.$ZodType | undefined; | ||||||||||||||||||
| /** JSON Schema property path for this schema */ | ||||||||||||||||||
| path?: (string | number)[] | undefined; | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
@@ -172,14 +175,7 @@ export function process<T extends schemas.$ZodType>( | |||||||||||||||||
| path: _params.path, | ||||||||||||||||||
| }; | ||||||||||||||||||
|
|
||||||||||||||||||
| const parent = schema._zod.parent as T; | ||||||||||||||||||
|
|
||||||||||||||||||
| if (parent) { | ||||||||||||||||||
| // schema was cloned from another schema | ||||||||||||||||||
| result.ref = parent; | ||||||||||||||||||
| process(parent, ctx, params); | ||||||||||||||||||
| ctx.seen.get(parent)!.isParent = true; | ||||||||||||||||||
| } else if (schema._zod.processJSONSchema) { | ||||||||||||||||||
| if (schema._zod.processJSONSchema) { | ||||||||||||||||||
| schema._zod.processJSONSchema(ctx, result.schema, params); | ||||||||||||||||||
| } else { | ||||||||||||||||||
| const _json = result.schema; | ||||||||||||||||||
|
|
@@ -189,6 +185,17 @@ export function process<T extends schemas.$ZodType>( | |||||||||||||||||
| } | ||||||||||||||||||
| processor(schema, ctx, _json, params); | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| const parent = schema._zod.parent as T; | ||||||||||||||||||
|
|
||||||||||||||||||
| if (parent) { | ||||||||||||||||||
| // Track parent separately from processor ref | ||||||||||||||||||
| result.parent = parent; | ||||||||||||||||||
| // Also set ref if processor didn't (for inheritance) | ||||||||||||||||||
| if (!result.ref) result.ref = parent; | ||||||||||||||||||
| process(parent, ctx, params); | ||||||||||||||||||
| ctx.seen.get(parent)!.isParent = true; | ||||||||||||||||||
| } | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| // metadata | ||||||||||||||||||
|
|
@@ -357,49 +364,89 @@ export function finalize<T extends schemas.$ZodType>( | |||||||||||||||||
| ctx: ToJSONSchemaContext, | ||||||||||||||||||
| schema: T | ||||||||||||||||||
| ): ZodStandardJSONSchemaPayload<T> { | ||||||||||||||||||
| // | ||||||||||||||||||
|
|
||||||||||||||||||
| // iterate over seen map; | ||||||||||||||||||
| const root = ctx.seen.get(schema); | ||||||||||||||||||
|
|
||||||||||||||||||
| if (!root) throw new Error("Unprocessed schema. This is a bug in Zod."); | ||||||||||||||||||
|
|
||||||||||||||||||
| // flatten _refs | ||||||||||||||||||
| // flatten refs - inherit properties from parent schemas | ||||||||||||||||||
| const flattenRef = (zodSchema: schemas.$ZodType) => { | ||||||||||||||||||
| const seen = ctx.seen.get(zodSchema)!; | ||||||||||||||||||
| const schema = seen.def ?? seen.schema; | ||||||||||||||||||
|
|
||||||||||||||||||
| const _cached = { ...schema }; | ||||||||||||||||||
| // already processed | ||||||||||||||||||
| if (seen.ref === null) return; | ||||||||||||||||||
|
|
||||||||||||||||||
| // already seen | ||||||||||||||||||
| if (seen.ref === null) { | ||||||||||||||||||
| return; | ||||||||||||||||||
| } | ||||||||||||||||||
| const schema = seen.def ?? seen.schema; | ||||||||||||||||||
| const _cached = { ...schema }; | ||||||||||||||||||
|
|
||||||||||||||||||
| // flatten ref if defined | ||||||||||||||||||
| const ref = seen.ref; | ||||||||||||||||||
| seen.ref = null; // prevent recursion | ||||||||||||||||||
| seen.ref = null; // prevent infinite recursion | ||||||||||||||||||
|
|
||||||||||||||||||
| if (ref) { | ||||||||||||||||||
| flattenRef(ref); | ||||||||||||||||||
|
|
||||||||||||||||||
| const refSeen = ctx.seen.get(ref)!; | ||||||||||||||||||
| const refSchema = refSeen.schema; | ||||||||||||||||||
|
|
||||||||||||||||||
| // merge referenced schema into current | ||||||||||||||||||
| const refSchema = ctx.seen.get(ref)!.schema; | ||||||||||||||||||
| if (refSchema.$ref && (ctx.target === "draft-07" || ctx.target === "draft-04" || ctx.target === "openapi-3.0")) { | ||||||||||||||||||
| // older drafts can't combine $ref with other properties | ||||||||||||||||||
| schema.allOf = schema.allOf ?? []; | ||||||||||||||||||
| schema.allOf.push(refSchema); | ||||||||||||||||||
| } else { | ||||||||||||||||||
| Object.assign(schema, refSchema); | ||||||||||||||||||
| Object.assign(schema, _cached); // prevent overwriting any fields in the original schema | ||||||||||||||||||
| } | ||||||||||||||||||
| // restore child's own properties (child wins) | ||||||||||||||||||
| Object.assign(schema, _cached); | ||||||||||||||||||
|
|
||||||||||||||||||
| const isParentRef = (zodSchema as any)._zod.parent === ref; | ||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using |
||||||||||||||||||
|
|
||||||||||||||||||
| // For parent chain, child is a refinement - remove parent-only properties | ||||||||||||||||||
| if (isParentRef) { | ||||||||||||||||||
| for (const key in schema) { | ||||||||||||||||||
| if (key === "$ref" || key === "allOf") continue; | ||||||||||||||||||
| if (!(key in _cached)) { | ||||||||||||||||||
| delete schema[key]; | ||||||||||||||||||
| } | ||||||||||||||||||
| } | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| // When ref was extracted to $defs, remove properties that match the definition | ||||||||||||||||||
| if (refSchema.$ref) { | ||||||||||||||||||
| for (const key in schema) { | ||||||||||||||||||
| if (key === "$ref" || key === "allOf") continue; | ||||||||||||||||||
| if (key in refSeen.def! && JSON.stringify(schema[key]) === JSON.stringify(refSeen.def![key])) { | ||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Comment on lines
+413
to
+416
|
||||||||||||||||||
| if (refSchema.$ref) { | |
| for (const key in schema) { | |
| if (key === "$ref" || key === "allOf") continue; | |
| if (key in refSeen.def! && JSON.stringify(schema[key]) === JSON.stringify(refSeen.def![key])) { | |
| if (refSchema.$ref && refSeen.def) { | |
| for (const key in schema) { | |
| if (key === "$ref" || key === "allOf") continue; | |
| if (key in refSeen.def && JSON.stringify(schema[key]) === JSON.stringify(refSeen.def[key])) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@colinhacks I'm getting this exact error when trying to upgrade from 4.2.1 to 4.3.5:
node_modules/.pnpm/zod@4.3.5/node_modules/zod/v4/core/to-json-schema.cjs:267
if (key in refSeen.def && JSON.stringify(schema[key]) === JSON.stringify(refSeen.def[key])) {
^
TypeError: Cannot use 'in' operator to search for 'id' in undefined
at flattenRef (node_modules/.pnpm/zod@4.3.5/node_modules/zod/v4/core/to-json-schema.cjs:267:29)
Maybe it should have the safety checks suggested by copilot?
I can confirm adding refSeen.def && in the beginning of line 416 fixed the issue for me.
PR: #5644
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The !seen.isParent guard was removed, doubling the overrideCount in tests. Is this intentional? If users were relying on overrides not being called for parent schemas (e.g., to avoid double-processing), this is a breaking change.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,19 @@ | ||
| import * as z from "zod/v4"; | ||
| import * as z from "./packages/zod/src/index.js"; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This looks like debug/exploration code that shouldn't be committed. Consider reverting this file or adding it to |
||
|
|
||
| z; | ||
| z.unknown() | ||
| .refine((val) => typeof val === "number") | ||
| .parse(1); | ||
| // Test: metadata order matters? | ||
|
|
||
| // Case 1: .meta() before .min() - reported as losing metadata | ||
| const schema1 = z.object({ | ||
| name: z.string().meta({ description: "first name" }).min(1), | ||
| }); | ||
|
|
||
| // Case 2: .meta() after .min() - reported as working | ||
| const schema2 = z.object({ | ||
| name: z.string().min(1).meta({ description: "A user name" }), | ||
| }); | ||
|
|
||
| console.log("Case 1 - .meta() before .min():"); | ||
| console.log(JSON.stringify(z.toJSONSchema(schema1), null, 2)); | ||
|
|
||
| console.log("\nCase 2 - .meta() after .min():"); | ||
| console.log(JSON.stringify(z.toJSONSchema(schema2), null, 2)); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The override count expectation changed from 6 to 12. This change reflects that the override function now runs on parent schemas as well, which were previously skipped (by checking
!seen.isParent). This doubles the count because each schema in the chain is now visited. While this aligns with the code change in the finalize function (removing theisParentcheck), consider adding a comment explaining why this number doubled to make the test more maintainable and document the expected behavior.