Add z.fromJSONSchema(), z.looseRecord(), z.xor()#5534
Conversation
colinhacks
commented
Dec 14, 2025
colinhacks
commented
- Initial version of z.fromJSONSchema
- Add z.fromJSONSchema
|
cc @pullfrog |
There was a problem hiding this comment.
Pull request overview
This PR adds three major features to Zod: z.fromJSONSchema() for converting JSON Schema to Zod schemas, z.looseRecord() for records that pass through non-matching keys, and exclusive unions (via z.union([...], { exclusive: true })) which require exactly one schema to match. The PR also refactors the toJSONSchema export to support the new exclusive union feature.
Key Changes:
- Implemented
fromJSONSchema()to convert JSON Schema (drafts 4, 7, 2020-12, and OpenAPI 3.0) to Zod schemas - Added
looseRecord()that passes through keys not matching the key pattern instead of erroring - Added exclusive union support via
{ exclusive: true }option inz.union()
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| play.ts | Demo/test code showing intersection and record usage examples |
| packages/zod/src/v4/mini/schemas.ts | Adds looseRecord function to mini API |
| packages/zod/src/v4/mini/external.ts | Updates toJSONSchema export path |
| packages/zod/src/v4/core/to-json-schema.ts | Updates union handling to use oneOf for exclusive unions |
| packages/zod/src/v4/core/schemas.ts | Implements exclusive union logic with new mode parameter for records |
| packages/zod/src/v4/core/index.ts | Removes toJSONSchema export from core index |
| packages/zod/src/v4/core/errors.ts | Adds error types for exclusive unions (multiple match vs no match) |
| packages/zod/src/v4/classic/tests/union.test.ts | Comprehensive tests for exclusive union behavior |
| packages/zod/src/v4/classic/tests/record.test.ts | Tests for looseRecord functionality |
| packages/zod/src/v4/classic/tests/from-json-schema.test.ts | Extensive tests for fromJSONSchema conversion |
| packages/zod/src/v4/classic/schemas.ts | Adds looseRecord function to classic API |
| packages/zod/src/v4/classic/from-json-schema.ts | Full implementation of JSON Schema to Zod converter |
| packages/zod/src/v4/classic/external.ts | Exports toJSONSchema and fromJSONSchema |
| packages/treeshake/zod-string.ts | Adds trailing newline |
| packages/treeshake/zod-locale.ts | New treeshake test for locale imports |
| packages/docs/content/api.mdx | Documents looseRecord feature |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
play.ts
Outdated
|
|
||
| const arg: Record<`sadf_${string}`, string> = { | ||
| sadf_asdf: "asdf", | ||
| aweqewr: "asdf", |
There was a problem hiding this comment.
The property aweqewr does not match the required pattern sadf_${string} according to the type annotation on line 29. TypeScript should flag this as an error. This appears to be test/demo code showing incorrect usage rather than correct usage.
| aweqewr: "asdf", | |
| // aweqewr: "asdf", // Removed: does not match pattern |
| const schema = z.union([z.string(), z.number()], { exclusive: true }); | ||
| expect(schema.parse("hello")).toBe("hello"); | ||
| expect(schema.parse(42)).toBe(42); | ||
| }); | ||
|
|
||
| test("exclusive union - zero matches fails", () => { | ||
| const schema = z.union([z.string(), z.number()], { exclusive: true }); |
There was a problem hiding this comment.
The PR title mentions z.xor() but only z.union([...], { exclusive: true }) is implemented. Consider either adding a dedicated z.xor() function for better discoverability, or updating the PR title to accurately reflect that exclusive unions are accessed via the union options parameter.
| const schema = z.union([z.string(), z.number()], { exclusive: true }); | |
| expect(schema.parse("hello")).toBe("hello"); | |
| expect(schema.parse(42)).toBe(42); | |
| }); | |
| test("exclusive union - zero matches fails", () => { | |
| const schema = z.union([z.string(), z.number()], { exclusive: true }); | |
| const schema = z.xor([z.string(), z.number()]); | |
| expect(schema.parse("hello")).toBe("hello"); | |
| expect(schema.parse(42)).toBe(42); | |
| }); | |
| test("exclusive union - zero matches fails", () => { | |
| const schema = z.xor([z.string(), z.number()]); |
| if (schema.anyOf && Array.isArray(schema.anyOf)) { | ||
| const options = schema.anyOf.map((s) => convertSchema(s, ctx)); | ||
| if (hasExplicitType) { | ||
| options.unshift(baseSchema); | ||
| } | ||
| baseSchema = z.union(options as [ZodType, ZodType, ...ZodType[]]); | ||
| } | ||
|
|
||
| // Handle oneOf - exclusive union (exactly one must match) | ||
| if (schema.oneOf && Array.isArray(schema.oneOf)) { | ||
| const options = schema.oneOf.map((s) => convertSchema(s, ctx)); | ||
| if (hasExplicitType) { | ||
| options.unshift(baseSchema); | ||
| } | ||
| baseSchema = z.union(options as [ZodType, ZodType, ...ZodType[]], { exclusive: true }); | ||
| } |
There was a problem hiding this comment.
When a schema has both anyOf and oneOf keywords, the current implementation processes both sequentially (lines 472-487). This means the oneOf processing would overwrite the anyOf result. According to JSON Schema specification, having both keywords is allowed and both should be satisfied. This should either be handled properly (e.g., by intersecting the results) or throw an error indicating that combining these keywords is not supported.
| // Expand type array into anyOf union | ||
| const typeSchemas = type.map((t) => { | ||
| const typeSchema: JSONSchema.JSONSchema = { ...schema, type: t }; |
There was a problem hiding this comment.
When expanding a type array into a union at line 156, the entire schema (including all constraints like minLength, maxLength, etc.) is spread into each type variant. This could lead to incorrect schemas if constraints are type-specific. For example, { type: ["string", "number"], minLength: 5 } would incorrectly apply minLength to the number variant. Consider only copying type-specific constraints to their respective variants.
| // Expand type array into anyOf union | |
| const typeSchemas = type.map((t) => { | |
| const typeSchema: JSONSchema.JSONSchema = { ...schema, type: t }; | |
| // Expand type array into anyOf union, filtering constraints to type-specific ones | |
| const typeSchemas = type.map((t) => { | |
| const typeSchema: JSONSchema.JSONSchema = filterSchemaForType(schema, t); |
| if (schema.patternProperties) { | ||
| // patternProperties: keys matching pattern must satisfy corresponding schema | ||
| // Use loose records so non-matching keys pass through | ||
| const patternProps = schema.patternProperties; | ||
| const patternKeys = Object.keys(patternProps); | ||
| const looseRecords: ZodType[] = []; | ||
|
|
||
| for (const pattern of patternKeys) { | ||
| const patternValue = convertSchema(patternProps[pattern] as JSONSchema.JSONSchema, ctx); | ||
| const keySchema = z.string().regex(new RegExp(pattern)); | ||
| looseRecords.push(z.looseRecord(keySchema, patternValue)); | ||
| } | ||
|
|
||
| // Build intersection: object schema + all pattern property records | ||
| const schemasToIntersect: ZodType[] = []; | ||
| if (Object.keys(shape).length > 0) { | ||
| // Use passthrough so patternProperties can validate additional keys | ||
| schemasToIntersect.push(z.object(shape).passthrough()); | ||
| } | ||
| schemasToIntersect.push(...looseRecords); | ||
|
|
||
| if (schemasToIntersect.length === 0) { | ||
| zodSchema = z.object({}).passthrough(); | ||
| } else if (schemasToIntersect.length === 1) { | ||
| zodSchema = schemasToIntersect[0]!; | ||
| } else { | ||
| // Chain intersections: (A & B) & C & D ... | ||
| let result = z.intersection(schemasToIntersect[0]!, schemasToIntersect[1]!); | ||
| for (let i = 2; i < schemasToIntersect.length; i++) { | ||
| result = z.intersection(result, schemasToIntersect[i]!); | ||
| } | ||
| zodSchema = result; | ||
| } | ||
| break; | ||
| } | ||
|
|
||
| // Handle additionalProperties | ||
| // In JSON Schema, additionalProperties defaults to true (allow any extra properties) | ||
| // In Zod, objects strip unknown keys by default, so we need to handle this explicitly | ||
| const objectSchema = z.object(shape); | ||
| if (schema.additionalProperties === false) { | ||
| // Strict mode - no extra properties allowed | ||
| zodSchema = objectSchema.strict(); | ||
| } else if (typeof schema.additionalProperties === "object") { | ||
| // Extra properties must match the specified schema | ||
| zodSchema = objectSchema.catchall(convertSchema(schema.additionalProperties as JSONSchema.JSONSchema, ctx)); | ||
| } else { | ||
| // additionalProperties is true or undefined - allow any extra properties (passthrough) | ||
| zodSchema = objectSchema.passthrough(); |
There was a problem hiding this comment.
When patternProperties is present, the code breaks out early (line 336), bypassing the additionalProperties handling logic. This means that if both patternProperties and additionalProperties are specified in a JSON Schema, the additionalProperties behavior will be ignored. Both should be handled together - additionalProperties controls keys that don't match any pattern in patternProperties.
| if (schema.patternProperties) { | |
| // patternProperties: keys matching pattern must satisfy corresponding schema | |
| // Use loose records so non-matching keys pass through | |
| const patternProps = schema.patternProperties; | |
| const patternKeys = Object.keys(patternProps); | |
| const looseRecords: ZodType[] = []; | |
| for (const pattern of patternKeys) { | |
| const patternValue = convertSchema(patternProps[pattern] as JSONSchema.JSONSchema, ctx); | |
| const keySchema = z.string().regex(new RegExp(pattern)); | |
| looseRecords.push(z.looseRecord(keySchema, patternValue)); | |
| } | |
| // Build intersection: object schema + all pattern property records | |
| const schemasToIntersect: ZodType[] = []; | |
| if (Object.keys(shape).length > 0) { | |
| // Use passthrough so patternProperties can validate additional keys | |
| schemasToIntersect.push(z.object(shape).passthrough()); | |
| } | |
| schemasToIntersect.push(...looseRecords); | |
| if (schemasToIntersect.length === 0) { | |
| zodSchema = z.object({}).passthrough(); | |
| } else if (schemasToIntersect.length === 1) { | |
| zodSchema = schemasToIntersect[0]!; | |
| } else { | |
| // Chain intersections: (A & B) & C & D ... | |
| let result = z.intersection(schemasToIntersect[0]!, schemasToIntersect[1]!); | |
| for (let i = 2; i < schemasToIntersect.length; i++) { | |
| result = z.intersection(result, schemasToIntersect[i]!); | |
| } | |
| zodSchema = result; | |
| } | |
| break; | |
| } | |
| // Handle additionalProperties | |
| // In JSON Schema, additionalProperties defaults to true (allow any extra properties) | |
| // In Zod, objects strip unknown keys by default, so we need to handle this explicitly | |
| const objectSchema = z.object(shape); | |
| if (schema.additionalProperties === false) { | |
| // Strict mode - no extra properties allowed | |
| zodSchema = objectSchema.strict(); | |
| } else if (typeof schema.additionalProperties === "object") { | |
| // Extra properties must match the specified schema | |
| zodSchema = objectSchema.catchall(convertSchema(schema.additionalProperties as JSONSchema.JSONSchema, ctx)); | |
| } else { | |
| // additionalProperties is true or undefined - allow any extra properties (passthrough) | |
| zodSchema = objectSchema.passthrough(); | |
| // Handle patternProperties and additionalProperties together | |
| let hasPatternProperties = !!schema.patternProperties; | |
| let patternProps = schema.patternProperties || {}; | |
| let patternKeys = Object.keys(patternProps); | |
| let looseRecords: ZodType[] = []; | |
| if (hasPatternProperties) { | |
| for (const pattern of patternKeys) { | |
| const patternValue = convertSchema(patternProps[pattern] as JSONSchema.JSONSchema, ctx); | |
| const keySchema = z.string().regex(new RegExp(pattern)); | |
| looseRecords.push(z.looseRecord(keySchema, patternValue)); | |
| } | |
| } | |
| // Build intersection: object schema + all pattern property records | |
| const schemasToIntersect: ZodType[] = []; | |
| if (Object.keys(shape).length > 0) { | |
| // Use passthrough so patternProperties can validate additional keys | |
| schemasToIntersect.push(z.object(shape).passthrough()); | |
| } | |
| schemasToIntersect.push(...looseRecords); | |
| let baseSchema: ZodType; | |
| if (schemasToIntersect.length === 0) { | |
| baseSchema = z.object({}).passthrough(); | |
| } else if (schemasToIntersect.length === 1) { | |
| baseSchema = schemasToIntersect[0]!; | |
| } else { | |
| // Chain intersections: (A & B) & C & D ... | |
| let result = z.intersection(schemasToIntersect[0]!, schemasToIntersect[1]!); | |
| for (let i = 2; i < schemasToIntersect.length; i++) { | |
| result = z.intersection(result, schemasToIntersect[i]!); | |
| } | |
| baseSchema = result; | |
| } | |
| // Handle additionalProperties | |
| // In JSON Schema, additionalProperties defaults to true (allow any extra properties) | |
| // In Zod, objects strip unknown keys by default, so we need to handle this explicitly | |
| if (schema.additionalProperties === false) { | |
| // Strict mode - no extra properties allowed | |
| zodSchema = baseSchema.strict(); | |
| } else if (typeof schema.additionalProperties === "object") { | |
| // Extra properties must match the specified schema | |
| zodSchema = baseSchema.catchall(convertSchema(schema.additionalProperties as JSONSchema.JSONSchema, ctx)); | |
| } else { | |
| // additionalProperties is true or undefined - allow any extra properties (passthrough) | |
| zodSchema = baseSchema.passthrough(); |
play.ts
Outdated
| const arg: Record<`sadf_${string}`, string> = { | ||
| sadf_asdf: "asdf", | ||
| aweqewr: "asdf", | ||
| }; |
There was a problem hiding this comment.
Unused variable arg.
| const arg: Record<`sadf_${string}`, string> = { | |
| sadf_asdf: "asdf", | |
| aweqewr: "asdf", | |
| }; |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR successfully implements fromJSONSchema() for converting JSON Schema to Zod and looseRecord() for partial key validation. All tests pass and the build succeeds. However, there are several concerns that should be addressed.
Critical issues:
- PR title mentions
z.xor()but it's not implemented separately (xor isz.union([...], { exclusive: true })) - Unrelated files included (play.ts, zod-locale.ts, zod-string.ts)
- Type safety compromised with excessive
as anycasts - Potential bugs in tuple validation and patternProperties handling
|
|
||
| function convertBaseSchema(schema: JSONSchema.JSONSchema, ctx: ConversionContext): ZodType { | ||
| // Handle unsupported features | ||
| // Note: oneOf is handled same as anyOf in convertSchema |
There was a problem hiding this comment.
Misleading comment: says 'oneOf is handled same as anyOf' but oneOf uses { exclusive: true } (lines 481-487) making it XOR-like (exactly one match), while anyOf allows multiple matches. Clarify this distinction.
| for (const pattern of patternKeys) { | ||
| const patternValue = convertSchema(patternProps[pattern] as JSONSchema.JSONSchema, ctx); | ||
| const keySchema = z.string().regex(new RegExp(pattern)); | ||
| looseRecords.push(z.looseRecord(keySchema, patternValue)); |
There was a problem hiding this comment.
Bug: When multiple patternProperties patterns match the same key, JSON Schema requires applying all matching schemas (intersection). Current implementation may only validate against one pattern. Need to handle overlapping patterns correctly.
| } | ||
| // Apply minItems/maxItems constraints to tuples | ||
| if (typeof schema.minItems === "number" || typeof schema.maxItems === "number") { | ||
| zodSchema = (zodSchema as any).refine( |
There was a problem hiding this comment.
Using .refine() for tuple minItems/maxItems produces poor error messages. Zod arrays have .min()/.max() but tuples don't. Consider documenting this limitation or finding a better approach. Same issue at line 409.
|
|
||
| // Apply metadata | ||
| if (schema.description) { | ||
| zodSchema = (zodSchema as any).describe(schema.description); |
There was a problem hiding this comment.
Type safety issue: (zodSchema as any).describe() and (zodSchema as any).default() lose type checking. Consider a helper function with proper types or type guards. Same at line 455.
|
Review complete! I've analyzed the implementation of Summary:
Main concerns:
The implementation is solid overall, but needs cleanup and bug fixes before merging. |
There was a problem hiding this comment.
Strong implementation with good test coverage. Several critical issues need attention: type array handling, infinite recursion risk, and composition logic. Also note: PR title mentions z.xor() but it's not implemented.
| if (Array.isArray(type)) { | ||
| // Expand type array into anyOf union | ||
| const typeSchemas = type.map((t) => { | ||
| const typeSchema: JSONSchema.JSONSchema = { ...schema, type: t }; |
There was a problem hiding this comment.
Critical - Potential infinite recursion: Spreading schema includes the original type array. If type: ['string', 'number'], this recursively calls convertBaseSchema with the same type array, causing infinite recursion. Fix: const { type: _, ...rest } = schema; const typeSchema = { ...rest, type: t };
| } | ||
|
|
||
| function convertBaseSchema(schema: JSONSchema.JSONSchema, ctx: ConversionContext): ZodType { | ||
| // Handle unsupported features |
There was a problem hiding this comment.
Misleading comment: Says 'oneOf is handled same as anyOf' but line 486 shows oneOf uses { exclusive: true } for exactly-one-match semantics (different from anyOf). Update to: 'oneOf is handled as exclusive union in convertSchema'
| const options = schema.anyOf.map((s) => convertSchema(s, ctx)); | ||
| if (hasExplicitType) { | ||
| options.unshift(baseSchema); | ||
| } |
There was a problem hiding this comment.
Logic issue: When a schema has explicit type AND anyOf, you unshift baseSchema into options. But if anyOf contains the same type, you get duplicate/conflicting constraints. Consider using intersection instead of union when hasExplicitType is true.
| if (schema.allOf && Array.isArray(schema.allOf)) { | ||
| let result = hasExplicitType ? baseSchema : convertSchema(schema.allOf[0]!, ctx); | ||
| const startIdx = hasExplicitType ? 0 : 1; | ||
| for (let i = startIdx; i < schema.allOf.length; i++) { |
There was a problem hiding this comment.
Missing edge case: allOf with zero elements will crash at schema.allOf[0]! when hasExplicitType is false. Add guard: if (!schema.allOf.length) return baseSchema;
| if (schemasToIntersect.length === 0) { | ||
| zodSchema = z.object({}).passthrough(); | ||
| } else if (schemasToIntersect.length === 1) { | ||
| zodSchema = schemasToIntersect[0]!; |
There was a problem hiding this comment.
Intersection chaining (A & B) & C can fail if schemas conflict. This same pattern repeats at line 500. Consider documenting the limitation or using a more robust composition strategy.
packages/docs/content/api.mdx
Outdated
| // { id?: string; name?: string; email?: string } | ||
| ``` | ||
|
|
||
| ### Loose records |
There was a problem hiding this comment.
Clarify when to use: Add note distinguishing from .passthrough(): 'Unlike passthrough (allows any extras), looseRecord only validates pattern-matching keys. Use passthrough for fixed properties + extras; looseRecord for pattern-based validation.'
| @@ -0,0 +1,5 @@ | |||
| import * as z from "zod"; | |||
There was a problem hiding this comment.
Unrelated file: This file is unrelated to fromJSONSchema/looseRecord/xor mentioned in PR title/description. Should be in separate commit or explained in PR body.
|
|
||
| const schema = z.string().min(5); | ||
| console.log(schema.parse("hi")); | ||
|
|
There was a problem hiding this comment.
Unrelated change: Whitespace-only change. Remove or explain why included in this PR.
|
cc @coderabbitai review this |
|
@colinhacks: I'll review the changes for you! ✅ Actions performedReview triggered.
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis pull request introduces several new features and refinements to Zod. A new Pre-merge checks❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (13)
packages/treeshake/zod-string.ts (1)
5-5: Unrelated change: Remove from PR.This whitespace-only change doesn't relate to the PR's stated objectives (fromJSONSchema, looseRecord, xor). Keep the PR focused.
packages/treeshake/zod-locale.ts (1)
1-5: Unrelated file: Remove from PR.This locale configuration file doesn't relate to the PR's stated objectives (fromJSONSchema, looseRecord, xor). If you need to test locale behavior, use
play.tsfor quick experiments or add proper test cases.Based on learnings, use
play.tsfor quick experimentation; use proper tests for all permanent test cases.packages/docs/content/api.mdx (4)
1852-1852: Clarify distinction from.passthrough().Add a note explaining when to use
looseRecordvs.passthrough(): "Unlike passthrough (which allows any extras), looseRecord only validates pattern-matching keys. Use passthrough for fixed properties + extras; looseRecord for pattern-based validation."
1864-1864: Unclear example: Show the error is thrown.The comment says "❌ value must be string" but doesn't demonstrate that an error is actually thrown. Consider using
expect(() => schema.parse({ S_name: 123 })).toThrow()style for clarity.
1869-1869: Add context: WHY useful with intersections.The docs say loose records are "particularly useful when combined with intersections" but don't explain why. Add: "This allows modeling multiple pattern properties from JSON Schema, where different patterns validate different key types."
1870-1870: Fix invalid syntax:z.intersection()only accepts 2 arguments.The code
z.intersection(A, B, C)won't work. Should bez.intersection(z.intersection(A, B), C)or use the.and()method.play.ts (1)
5-32: Unrelated file: Remove playground code from PR.The
play.tsfile is for quick experimentation and shouldn't be included in pull requests. Remove this file or revert these changes.Also note: Line 31 has a property
aweqewrthat doesn't match the required patternsadf_${string}according to the type annotation on line 29. TypeScript should flag this as an error.Based on learnings, use
play.tsfor quick experimentation; use proper tests for all permanent test cases.packages/zod/src/v4/classic/tests/union.test.ts (1)
183-298: Comprehensive test coverage for exclusive unions.The test suite thoroughly covers all scenarios for exclusive unions: exactly one match, zero matches, multiple matches, overlapping/non-overlapping schemas, and async parsing. Well done!
Note: The PR title mentions
z.xor()but the implementation usesz.union([...], { exclusive: true }). Consider either adding a dedicatedz.xor()convenience function for better discoverability, or updating the PR title to accurately reflect the implementation approach.packages/zod/src/v4/classic/tests/from-json-schema.test.ts (1)
200-207: Consider adding explicit exclusive matching verification.The "oneOf schema (exclusive union)" test validates that single matches work, but doesn't explicitly test that multiple matches fail (the defining characteristic of oneOf vs anyOf). While line 228-247 tests this behavior in a more complex scenario, the basic oneOf test would benefit from a case that demonstrates exclusive matching.
Based on learnings, past review feedback suggested verifying that oneOf fails when multiple schemas match.
Consider adding a test case like:
test("oneOf schema (exclusive union)", () => { const schema = fromJSONSchema({ oneOf: [{ type: "string" }, { type: "number" }], }); expect(schema.parse("hello")).toBe("hello"); expect(schema.parse(42)).toBe(42); expect(() => schema.parse(true)).toThrow(); // Test exclusive matching: schema that could match multiple branches const ambiguousSchema = fromJSONSchema({ oneOf: [ { type: "string", minLength: 3 }, { type: "string", maxLength: 10 } ] }); // Should pass: only matches first (length >= 3, no maxLength constraint violated) expect(ambiguousSchema.parse("hi!")).toBe("hi!"); // Should fail: matches both (3 <= length <= 10) expect(() => ambiguousSchema.parse("hello")).toThrow(); });packages/zod/src/v4/classic/schemas.ts (1)
1446-1458: Add JSDoc documentation for the new public API.The
looseRecordfunction is missing JSDoc documentation. Consider adding comprehensive documentation that explains:
- What loose mode does and how it differs from the default record behavior
- When developers should use
looseRecordvsrecord- Parameter descriptions
- A usage example
Based on learnings, this aligns with past review feedback about missing documentation.
Example:
+/** + * Creates a loose record schema that validates matching keys but passes through non-matching keys unchanged. + * Unlike `record()`, which validates all keys, `looseRecord()` only applies validation to keys that match + * the keyType pattern, allowing other keys to pass through without validation. + * + * @param keyType - Schema for validating record keys (e.g., z.string(), z.enum(['a', 'b'])) + * @param valueType - Schema for validating record values + * @param params - Optional parameters for error messages + * @returns A ZodRecord schema with loose mode enabled + * + * @example + * const schema = z.looseRecord(z.string(), z.number()); + * schema.parse({ a: 1, b: 2, other: 'string' }); // passes: non-matching keys allowed + */ export function looseRecord<Key extends core.$ZodRecordKey, Value extends core.SomeType>(packages/zod/src/v4/classic/from-json-schema.ts (3)
48-58: OpenAPI 3.0 ref resolution might not work.The ref resolution only handles
#/$defs/...or#/definitions/...paths. OpenAPI 3.0 typically uses#/components/schemas/...for refs. If someone passes an OpenAPI 3.0 schema with refs, this will throw "Reference not found".Consider adding OpenAPI 3.0 support:
const defsKey = ctx.version === "draft-2020-12" ? "$defs" : "definitions"; if (path[0] === defsKey) { const key = path[1]; if (!key || !ctx.defs[key]) { throw new Error(`Reference not found: ${ref}`); } return ctx.defs[key]!; } + // OpenAPI 3.0: #/components/schemas/... + if (ctx.version === "openapi-3.0" && path[0] === "components" && path[1] === "schemas") { + const key = path[2]; + if (!key || !ctx.defs[key]) { + throw new Error(`Reference not found: ${ref}`); + } + return ctx.defs[key]!; + } throw new Error(`Reference not found: ${ref}`);You'd also need to update line 494 to pull defs from
schema.components?.schemasfor OpenAPI 3.0.
301-336: patternProperties breaks out early, skipping additionalProperties.When
patternPropertiesis present, the codebreaks at line 335, which means theadditionalPropertieshandling below (lines 338-351) never runs. In JSON Schema, both keywords can coexist -additionalPropertiescontrols keys that don't match any pattern OR any explicit property.The
additionalPropertieslogic should be integrated into thepatternPropertiesblock. This is a semantic mismatch with JSON Schema behavior.if (schemasToIntersect.length === 0) { zodSchema = z.object({}).passthrough(); } else if (schemasToIntersect.length === 1) { zodSchema = schemasToIntersect[0]!; } else { // Chain intersections: (A & B) & C & D ... let result = z.intersection(schemasToIntersect[0]!, schemasToIntersect[1]!); for (let i = 2; i < schemasToIntersect.length; i++) { result = z.intersection(result, schemasToIntersect[i]!); } zodSchema = result; } + // Note: additionalProperties handling is complex with patternProperties + // For now, patternProperties mode uses passthrough behavior break;At minimum, add a TODO or throw an error when both are specified to avoid silent incorrect behavior.
447-458: Both anyOf and oneOf can appear together - current code handles sequentially.If a schema has both
anyOfANDoneOf, the code processes them one after another. The second one wraps/intersects with the result of the first. This might not match JSON Schema semantics where both constraints should independently apply to the input.This is a pretty edge-case scenario, but worth a comment or throwing an error for unsupported combinations.
+ // Note: Having both anyOf and oneOf in the same schema is unusual. + // We process them sequentially, which may not match strict JSON Schema semantics. // Handle anyOf - wrap base schema with union if (schema.anyOf && Array.isArray(schema.anyOf)) {
🧹 Nitpick comments (1)
packages/zod/src/v4/classic/from-json-schema.ts (1)
3-3: Unused imports.
ZodNumberandZodStringare imported but only used as type annotations. Since they're only used in type positions, you could change toimport typefor cleaner tree-shaking, though this isn't blocking.-import type { ZodNumber, ZodString, ZodType } from "./schemas.js"; +import type { ZodType } from "./schemas.js"; +import type { ZodNumber, ZodString } from "./schemas.js";Or just keep them as-is since they're type-only anyway.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
packages/docs/content/api.mdx(1 hunks)packages/treeshake/zod-locale.ts(1 hunks)packages/treeshake/zod-string.ts(1 hunks)packages/zod/src/v4/classic/external.ts(1 hunks)packages/zod/src/v4/classic/from-json-schema.ts(1 hunks)packages/zod/src/v4/classic/schemas.ts(1 hunks)packages/zod/src/v4/classic/tests/from-json-schema.test.ts(1 hunks)packages/zod/src/v4/classic/tests/record.test.ts(1 hunks)packages/zod/src/v4/classic/tests/union.test.ts(1 hunks)packages/zod/src/v4/core/errors.ts(1 hunks)packages/zod/src/v4/core/index.ts(0 hunks)packages/zod/src/v4/core/schemas.ts(6 hunks)packages/zod/src/v4/core/to-json-schema.ts(1 hunks)packages/zod/src/v4/mini/external.ts(1 hunks)packages/zod/src/v4/mini/schemas.ts(1 hunks)play.ts(1 hunks)
💤 Files with no reviewable changes (1)
- packages/zod/src/v4/core/index.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,mjs,cjs,ts,mts,cts}
📄 CodeRabbit inference engine (AGENTS.md)
Use ES modules throughout ("type": "module")
Files:
packages/treeshake/zod-locale.tspackages/zod/src/v4/mini/external.tspackages/zod/src/v4/classic/tests/union.test.tspackages/zod/src/v4/classic/from-json-schema.tspackages/treeshake/zod-string.tspackages/zod/src/v4/classic/external.tspackages/zod/src/v4/classic/tests/from-json-schema.test.tspackages/zod/src/v4/classic/schemas.tspackages/zod/src/v4/core/to-json-schema.tspackages/zod/src/v4/classic/tests/record.test.tspackages/zod/src/v4/core/schemas.tspackages/zod/src/v4/core/errors.tspackages/zod/src/v4/mini/schemas.tsplay.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Don't skip tests due to type issues - fix the types instead in TypeScript files
Useutil.defineLazy()for computed properties to avoid circular dependencies in TypeScript
Files:
packages/treeshake/zod-locale.tspackages/zod/src/v4/mini/external.tspackages/zod/src/v4/classic/tests/union.test.tspackages/zod/src/v4/classic/from-json-schema.tspackages/treeshake/zod-string.tspackages/zod/src/v4/classic/external.tspackages/zod/src/v4/classic/tests/from-json-schema.test.tspackages/zod/src/v4/classic/schemas.tspackages/zod/src/v4/core/to-json-schema.tspackages/zod/src/v4/classic/tests/record.test.tspackages/zod/src/v4/core/schemas.tspackages/zod/src/v4/core/errors.tspackages/zod/src/v4/mini/schemas.tsplay.ts
**/*.{js,mjs,cjs,ts,mts,cts,tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,mjs,cjs,ts,mts,cts,tsx,jsx}: No log statements (console.log,debugger) in tests or production code
Parameter reassignment is allowed for performance optimization in TypeScript/JavaScript code
Files:
packages/treeshake/zod-locale.tspackages/zod/src/v4/mini/external.tspackages/zod/src/v4/classic/tests/union.test.tspackages/zod/src/v4/classic/from-json-schema.tspackages/treeshake/zod-string.tspackages/zod/src/v4/classic/external.tspackages/zod/src/v4/classic/tests/from-json-schema.test.tspackages/zod/src/v4/classic/schemas.tspackages/zod/src/v4/core/to-json-schema.tspackages/zod/src/v4/classic/tests/record.test.tspackages/zod/src/v4/core/schemas.tspackages/zod/src/v4/core/errors.tspackages/zod/src/v4/mini/schemas.tsplay.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
All tests must be written in TypeScript; never use JavaScript for tests
Files:
packages/zod/src/v4/classic/tests/union.test.tspackages/zod/src/v4/classic/tests/from-json-schema.test.tspackages/zod/src/v4/classic/tests/record.test.ts
🧠 Learnings (2)
📚 Learning: 2025-11-25T20:47:30.768Z
Learnt from: CR
Repo: colinhacks/zod PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T20:47:30.768Z
Learning: Use `play.ts` for quick experimentation; use proper tests for all permanent test cases
Applied to files:
packages/treeshake/zod-locale.tsplay.ts
📚 Learning: 2025-11-25T20:47:30.768Z
Learnt from: CR
Repo: colinhacks/zod PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T20:47:30.768Z
Learning: Applies to **/*.{ts,tsx} : Don't skip tests due to type issues - fix the types instead in TypeScript files
Applied to files:
packages/zod/src/v4/classic/tests/union.test.tsplay.ts
🧬 Code graph analysis (4)
packages/treeshake/zod-locale.ts (1)
packages/zod/src/index.ts (1)
z(3-3)
packages/zod/src/v4/classic/tests/from-json-schema.test.ts (2)
packages/zod/src/v4/classic/from-json-schema.ts (1)
fromJSONSchema(487-505)packages/zod/src/v4/classic/external.ts (1)
fromJSONSchema(33-33)
packages/zod/src/v4/core/schemas.ts (2)
packages/zod/src/v3/helpers/parseUtil.ts (1)
ParseContext(53-64)packages/zod/src/v4/core/core.ts (1)
input(107-107)
play.ts (3)
packages/zod/src/v4/core/core.ts (3)
input(107-107)output(108-108)output(110-110)packages/zod/src/v3/types.ts (2)
input(52-52)output(53-53)packages/zod/src/v4/classic/compat.ts (2)
output(8-8)output(10-10)
🪛 ast-grep (0.40.0)
packages/zod/src/v4/classic/from-json-schema.ts
[warning] 241-241: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(schema.pattern)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
[warning] 310-310: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(pattern)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🔇 Additional comments (16)
packages/zod/src/v4/mini/external.ts (1)
24-24: LGTM: Clean export reorganization.Moving
toJSONSchemato a dedicated module export improves modularity and keeps the codebase organized.packages/zod/src/v4/classic/tests/record.test.ts (1)
490-525: Excellent test coverage for looseRecord.Both tests thoroughly validate the new looseRecord functionality:
- Ensures matching keys are validated while non-matching keys pass through
- Tests complex intersections with multiple loose record patterns
The tests cover all the important edge cases and demonstrate the practical use of the feature.
packages/zod/src/v4/core/to-json-schema.ts (1)
333-341: LGTM: Correct JSON Schema generation for exclusive unions.The updated logic correctly emits
oneOfwhen either:
- The union is explicitly exclusive (
def.exclusive), or- The union has a discriminator (which implies mutual exclusivity)
This aligns perfectly with the new exclusive union feature and follows JSON Schema semantics where
oneOfenforces exactly one match.packages/zod/src/v4/classic/external.ts (1)
32-33: LGTM! Clean export reorganization.The re-export changes look good. Moving
toJSONSchemato a dedicated module and adding the newfromJSONSchemaexport follows good separation of concerns and makes the API surface clearer.packages/zod/src/v4/classic/tests/from-json-schema.test.ts (2)
24-35: Excellent pattern validation coverage!The pattern and patternProperties tests are well-designed. Particularly appreciate:
- Lines 24-35 explicitly verify that JSON Schema patterns are not implicitly anchored (important distinction from some regex implementations)
- Lines 326-387 cover single, multiple, and overlapping pattern properties
- Good comments explaining expected behavior (e.g., line 340-341)
Also applies to: 326-387
1-537: Comprehensive test coverage - nice work!The test suite provides excellent coverage of JSON Schema features:
- ✅ All primitive types with constraints
- ✅ Complex types (objects, arrays, tuples)
- ✅ Composition keywords (anyOf, allOf, oneOf)
- ✅ References (local, circular with lazy)
- ✅ Pattern properties (single, multiple, overlapping)
- ✅ Error cases for unsupported features
- ✅ Version detection across multiple JSON Schema drafts
- ✅ OpenAPI 3.0 nullable support
The tests are well-structured with descriptive names and cover both happy paths and error scenarios.
packages/zod/src/v4/core/schemas.ts (5)
2020-2020: Nice addition of the exclusive flag.Clean addition to the union definition. This aligns well with JSON Schema's
oneOfsemantics where exactly one schema must match.
2068-2101: Exclusive union handler looks solid.The logic correctly handles the three cases:
- Single match → success
- No matches → standard invalid_union error with all errors
- Multiple matches → exclusive union failure with
inclusive: falseOne small thing - when there are multiple matches, the
errorsarray is empty. This is intentional for the type system, but users debugging might appreciate knowing which schemas matched. Not blocking, just something to consider for future DX improvements.
2152-2163: Short-circuit suppression for exclusive mode is correct.Good call on only short-circuiting when NOT exclusive. For exclusive unions, we need to validate all options to ensure exactly one matches.
2578-2579: JSDoc added as requested in past reviews - nice.The documentation clarifies the behavior difference between strict and loose modes.
2707-2721: Loose mode pass-through is well-tested—looks good.The tests comprehensively cover loose mode behavior: non-matching keys pass through unvalidated while matching keys still get validated. This is the intended design for JSON Schema
patternPropertiessupport, and it's working correctly. The behavior is documented and tested thoroughly, so no action needed here.packages/zod/src/v4/classic/from-json-schema.ts (3)
152-165: Potential infinite recursion when expanding type arrays.When
typeis an array like["string", "number"], you spread the entire schema including thetypearray itself into each variant. IfconvertBaseSchemais called recursively with this structure, it could loop infinitely.Looking more closely... actually you're setting
type: twhich overwrites the array with a single string. So this should be fine. The spread happens first, thentype: toverwrites.Actually wait - the concern from past reviews is valid. When you do
{ ...schema, type: t }, thetypeproperty gets the single stringt, not the array. So this is safe. My bad, disregard!
461-472: Empty allOf array edge case.Nice handling of the empty
allOfcase at line 462-463. Returns the base schema orz.any()depending on whether there's an explicit type.
487-505: Clean entry point.The main
fromJSONSchemafunction is well-structured - handles boolean schemas upfront, detects version, initializes context, and delegates toconvertSchema.packages/zod/src/v4/core/errors.ts (2)
65-81: Nice refactor to discriminated union for invalid_union issues.Breaking this into
NoMatchandMultipleMatchvariants withinclusiveas the discriminant is clean. TypeScript can now narrow properly:
inclusive?: true(or undefined) → no matches,errorscontains per-option issuesinclusive: false→ multiple matches (exclusive union failure),errorsis emptyThis pairs well with the
handleExclusiveUnionResultsfunction in schemas.ts.
270-273: Error formatting handles empty errors array correctly.The check
issue.errors.lengthat line 272 will be falsy for theMultipleMatchvariant (whereerrors: []), so it won't try to recursively process an empty array.
| if (schema.pattern) { | ||
| // JSON Schema patterns are not implicitly anchored (match anywhere in string) | ||
| stringSchema = stringSchema.regex(new RegExp(schema.pattern)); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the file and surrounding context
cat -n packages/zod/src/v4/classic/from-json-schema.ts | sed -n '230,260p'Repository: colinhacks/zod
Length of output: 1292
🏁 Script executed:
# Search for any existing regex validation or safety measures in the codebase
rg -i "redog|safe-regex|regex.*timeout|pattern.*validat" --type ts --type jsRepository: colinhacks/zod
Length of output: 468
🏁 Script executed:
# Check if there are any comments or documentation about schema trust assumptions
rg -B5 -A5 "schema\.pattern" packages/zod/src/v4/classic/from-json-schema.tsRepository: colinhacks/zod
Length of output: 1159
🏁 Script executed:
# Look for how schemas are typically loaded/used in tests or examples
fd -e ts -e js | xargs rg -l "from-json-schema|fromJsonSchema" | head -10Repository: colinhacks/zod
Length of output: 156
🌐 Web query:
ReDoS attacks JavaScript RegExp prevention best practices safe-regex
💡 Result:
Below are concise best practices to prevent JavaScript ReDoS (Regular Expression Denial of Service), plus detection/mitigation tools.
Summary best practices
- Avoid running untrusted/user-supplied regexes; prefer literal text search or restricted patterns. [1][2]
- Limit input size and enforce sane length/complexity limits before matching. [1][4]
- Avoid vulnerable constructs: nested or adjacent quantifiers (e.g. (a+)+), ambiguous alternation (e.g. (a|aa)+), and backtracking‑heavy patterns. Rewrite to be deterministic when possible. [1][2]
- Prefer safe engines that guarantee linear-time matching (RE2 or bindings) when you must accept complex/untrusted patterns. Use node-re2 / re2-wasm etc. as a drop‑in safe alternative. [4][7]
- If you must use backtracking engine (native RegExp), add runtime protections: execute regex work in a worker/isolated process and enforce timeouts or input-size cutoffs so one bad input can’t hang the main thread. [1][3]
- Use atomic/possessive techniques when supported (note: JavaScript lacks native atomic groups/possessive quantifiers; there are workarounds but they’re complex). Prefer engine change if you need these features for safety. [6][8]
Detection and testing tools
- safe-regex / safe-regex2 — quick heuristic checker (may have false positives/negatives). Good for CI pre-checks. [2][5]
- redos-detector / vuln-regex-detector — stronger analyzers that attempt to detect exploitable patterns and produce attack strings. Use these for deeper analysis. [3][1]
- Static review / unit tests — include long “evil” test strings (generated or from detectors) in CI to verify performance. [3]
Practical mitigation recipe
- Static-check all regexes at authoring/CI time (safe-regex / redos-detector). Flag anything unsafe. [2][3]
- Reject or sandbox any user-supplied regexes; if allowed, run them with an engine like RE2 or in an isolated worker with time/cpu limits. [4][3]
- Enforce input length limits + deploy runtime timeouts for regex operations that might run on large inputs. [1][3]
- When possible, refactor fragile regexes to deterministic alternatives (explicit parsing, finite-state logic, or RE2-compatible patterns). [6][7]
References (examples)
- OWASP: ReDoS / overly permissive regex guidance. [1]
- safe-regex / safe-regex2 (heuristic checker). [2][5]
- redos-detector / vuln-regex-detector (stronger detectors, attack-string generation). [3][1]
- RE2 (node-re2 / re2-wasm) — linear-time regex engine recommended to eliminate ReDoS risk. [4][7]
If you want, I can:
- scan a codebase or a list of regexes with redos-detector and report any unsafe patterns; or
- show concrete rewrites of a specific vulnerable regex you paste here.
🏁 Script executed:
# Check documentation and comments about schema sources
rg -B10 "export.*fromJsonSchema|export.*from-json-schema" packages/zod/src/v4/classic/ -A5Repository: colinhacks/zod
Length of output: 1039
🏁 Script executed:
# Look at test cases to understand typical usage patterns
cat -n packages/zod/src/v4/classic/tests/from-json-schema.test.ts | head -100Repository: colinhacks/zod
Length of output: 3953
🏁 Script executed:
# Check for any README or docs mentioning security or trusted schemas
fd -e md -e txt | xargs rg -i "json schema|pattern|regex|trusted|security" 2>/dev/null | grep -i "from.*json\|pattern\|untrusted" | head -20Repository: colinhacks/zod
Length of output: 1683
🏁 Script executed:
# Search for how zod handles user-supplied regexes elsewhere
rg -B3 -A3 "\.regex\(" packages/zod/src/ --type ts | head -80Repository: colinhacks/zod
Length of output: 6189
🏁 Script executed:
# Check the function signature and documentation
cat -n packages/zod/src/v4/classic/from-json-schema.ts | head -50Repository: colinhacks/zod
Length of output: 1932
🏁 Script executed:
# Look for any JSDoc or comments about schema input expectations
rg -B20 "export.*function.*fromJSONSchema|export.*const.*fromJSONSchema" packages/zod/src/v4/classic/from-json-schema.ts | head -40Repository: colinhacks/zod
Length of output: 630
Document that fromJSONSchema expects trusted schema sources to mitigate ReDoS risk.
Constructing RegExp from untrusted schema.pattern values (line 242) and pattern properties (used in patternProperties handling) can enable ReDoS attacks. Since JSON Schema conversion typically happens at build/startup time with trusted schemas, document this assumption clearly. If untrusted schemas must be supported, consider adding pattern complexity validation (e.g., safe-regex package) or wrapping regex execution with timeouts.
🧰 Tools
🪛 ast-grep (0.40.0)
[warning] 241-241: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(schema.pattern)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🤖 Prompt for AI Agents
In packages/zod/src/v4/classic/from-json-schema.ts around lines 240 to 242, the
code constructs RegExp instances directly from schema.pattern (and likewise from
patternProperties elsewhere), which is unsafe for untrusted input; update the
codebase and docs to state clearly that fromJSONSchema expects trusted schema
sources (e.g., build/startup-time schemas) and will not sanitize malicious
patterns, and if you need to support untrusted schemas either validate pattern
complexity with a library like safe-regex (or similar) before constructing the
RegExp, or implement a guarded execution strategy (e.g., timeouts/worker
isolation) when running regexes; also add a short note to the function JSDoc and
README and add a comment near the RegExp construction mentioning the ReDoS risk
and the chosen mitigation.
| export function looseRecord<Key extends core.$ZodRecordKey, Value extends SomeType>( | ||
| keyType: Key, | ||
| valueType: Value, | ||
| params?: string | core.$ZodRecordParams | ||
| ): ZodMiniRecord<Key, Value> { | ||
| return new ZodMiniRecord({ | ||
| type: "record", | ||
| keyType, | ||
| valueType: valueType as any as core.$ZodType, | ||
| mode: "loose", | ||
| ...util.normalizeParams(params), | ||
| }) as any; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add JSDoc documentation to match the classic API.
The looseRecord function in the mini API is missing JSDoc documentation. For API consistency, this should have the same level of documentation as the classic version. Consider adding comprehensive JSDoc that explains the loose mode behavior and when to use this function.
Refer to the documentation suggestion for the classic version in packages/zod/src/v4/classic/schemas.ts for consistency.
🤖 Prompt for AI Agents
In packages/zod/src/v4/mini/schemas.ts around lines 1089 to 1101, add JSDoc to
the looseRecord export to match the classic API: document the function
signature, generic Key/Value parameters, explain "loose" mode semantics (keys
coerce/allow unknown extra keys or similar behavior used by classic version),
describe params argument (string or $ZodRecordParams) and returned ZodMiniRecord
type, and include short usage examples and notes about when to use looseRecord
versus strict records; mirror wording and format from
packages/zod/src/v4/classic/schemas.ts for consistency.
colinhacks
force-pushed
the
from-json-schema
branch
from
d89bdc1 to
99984e9
Compare
|
cc @pullfrog review with a focus on the XOR stuff |
There was a problem hiding this comment.
Focused review on XOR implementation - looks solid overall with good test coverage. Found a few minor considerations around code clarity and edge cases.
| for (const option of def.options) { | ||
| const result = option._zod.run( | ||
| { | ||
| value: payload.value, |
There was a problem hiding this comment.
Observation: XOR evaluates all options - Unlike regular unions which early-return on first success (line 2111 in original), XOR must evaluate all options to detect multiple matches. Consider adding a comment here explaining why we can't early-return, to make the intentional difference clear.
| final.issues.push({ | ||
| code: "invalid_union", | ||
| input: final.value, | ||
| inst, |
There was a problem hiding this comment.
Empty errors array for multiple matches - When multiple options match, you're correctly using inclusive: false with an empty errors array to distinguish from zero-match failures. This is semantically correct but might be surprising to error consumers. Verify that error formatters/locales handle this case properly (checking inclusive field when errors is empty).
| inst: $ZodUnion, | ||
| ctx?: ParseContext | ||
| ) { | ||
| const successes = results.filter((r) => r.issues.length === 0); |
There was a problem hiding this comment.
Clean XOR implementation - The logic here is clear: filter successes, handle exactly-one vs zero vs multiple. Nice separation of concerns with the dedicated handleExclusiveUnionResults function.
| const element = convertSchema(items as JSONSchema.JSONSchema, ctx); | ||
| let arraySchema = z.array(element); | ||
|
|
||
| // Apply constraints |
There was a problem hiding this comment.
Perfect mapping of JSON Schema oneOf to z.xor() - This correctly implements JSON Schema's 'exactly one' semantics. The intersection handling for when hasExplicitType is also correct.
| stringSchema = stringSchema.check(z.e164()); | ||
| } else if (format === "jwt") { | ||
| stringSchema = stringSchema.check(z.jwt()); | ||
| } else if (format === "emoji") { |
There was a problem hiding this comment.
Pattern behavior differs from Zod regex() - Good comment! JSON Schema patterns match anywhere (like JS .match()), while Zod's .regex() is implicitly anchored (like .test() with ^...$). Since you're passing the pattern directly to new RegExp(), this correctly preserves JSON Schema semantics. Worth noting in docs that users migrating patterns from Zod to JSON Schema may need to adjust.
packages/docs/content/api.mdx
Outdated
| z.object({ type: z.literal("card"), cardNumber: z.string() }), | ||
| z.object({ type: z.literal("bank"), accountNumber: z.string() }), | ||
| ]); | ||
|
|
There was a problem hiding this comment.
Excellent XOR documentation - The overlapping example clearly demonstrates the failure case. One enhancement: consider showing what the actual error message looks like so users know how to debug XOR failures (e.g., does it say 'multiple matches' or something else?).
|
✅ Review Complete - XOR Implementation Focus Reviewed the PR with specific attention to the XOR functionality as requested. Here's what I found: XOR Implementation (z.xor())
JSON Schema Integration
Documentation & Tests
Overall: The XOR implementation is well-designed and correctly implements exclusive union semantics. The integration with fromJSONSchema properly maps JSON Schema's oneOf behavior. No blocking issues found. |
colinhacks
changed the title
z.fromJSONSchema(), z.looseRecord(),z.xor()z.fromJSONSchema(), z.looseRecord(), z.xor()
|
The release notes use z.xor(
z.object({ type: "user", name: z.string() }),
z.object({ type: "admin", role: z.string() })
)...whicha appears to be an error? |
3 participants
