-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Fix Zod lazy cyclical mutual recursion #5373
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThis pull request adds a test for mutual recursion with cyclical data using z.lazy and implements a memoization mechanism in lazy schema parsing. A new test scenario creates forward-declared lazy schemas that reference each other and verifies cyclical links are preserved during parsing. On the implementation side, a WeakMap-based caching layer is introduced to store and reuse parse results for identical object payloads across repeated evaluations of the same lazy schema instance, using a new LAZY_PARSE_MEMO symbol on the parse context. Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/zod/src/v4/classic/tests/lazy.test.ts
(1 hunks)packages/zod/src/v4/core/schemas.ts
(2 hunks)
🧰 Additional context used
📓 Path-based instructions (15)
**/*.{js,jsx,ts,tsx,mjs,cjs,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Enforce line width of 120 characters via Biome formatting
Files:
packages/zod/src/v4/core/schemas.ts
packages/zod/src/v4/classic/tests/lazy.test.ts
**/*.{js,jsx,ts,tsx,mjs,cjs}
📄 CodeRabbit inference engine (CLAUDE.md)
Use ES5-style trailing commas in JavaScript/TypeScript code
Files:
packages/zod/src/v4/core/schemas.ts
packages/zod/src/v4/classic/tests/lazy.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}
: Allow the any type in TypeScript (noExplicitAny off)
Allow non-null assertions in TypeScript (noNonNullAssertion off)
Write TypeScript to pass strict mode with exactOptionalPropertyTypes enabled
Use NodeNext module resolution semantics for imports in TypeScript
Target ES2020 language features in TypeScript source
Files:
packages/zod/src/v4/core/schemas.ts
packages/zod/src/v4/classic/tests/lazy.test.ts
**/*.{ts,tsx,js,jsx,mjs,cjs}
📄 CodeRabbit inference engine (CLAUDE.md)
Allow parameter reassignment for performance-sensitive code (noParameterAssign off)
Files:
packages/zod/src/v4/core/schemas.ts
packages/zod/src/v4/classic/tests/lazy.test.ts
**/*.{js,mjs,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
**/*.{js,mjs,ts,tsx}
: Use .js extensions in import specifiers (e.g., import { z } from "./index.js")
Don’t use require(); use ESM import statements
Files:
packages/zod/src/v4/core/schemas.ts
packages/zod/src/v4/classic/tests/lazy.test.ts
**/*.{js,mjs,cjs,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/guidelines.mdc)
Do not leave log statements (e.g., console.log, debugger) in tests or production code
Files:
packages/zod/src/v4/core/schemas.ts
packages/zod/src/v4/classic/tests/lazy.test.ts
packages/zod/src/v4/core/{schemas.ts,core.ts}
📄 CodeRabbit inference engine (.cursor/rules/zod-internals.mdc)
Use the custom constructor system via core.$constructor() and initialize instances with $ZodType.init() when creating schemas
Files:
packages/zod/src/v4/core/schemas.ts
packages/zod/src/v4/core/schemas.ts
📄 CodeRabbit inference engine (.cursor/rules/zod-internals.mdc)
packages/zod/src/v4/core/schemas.ts
: Wrapper schemas (e.g., $ZodOptional, $ZodNullable, $ZodReadonly) must pass through internal properties from their inner type using util.defineLazy for propValues, values, optin, and optout
Define computed internal properties using util.defineLazy() to avoid circular dependencies
Implement schema parse functions following the standard structure: type check, push invalid_type issue on mismatch, optionally coerce/transform, and return payload
For $ZodDiscriminatedUnion, compute and merge propValues lazily from options; ensure each option provides the discriminator key and that values are unique
Ensure readonly wrapper types (e.g., $ZodReadonly) pass through values for discriminator support in unions
Files:
packages/zod/src/v4/core/schemas.ts
packages/zod/src/v4/core/{schemas.ts,checks.ts}
📄 CodeRabbit inference engine (.cursor/rules/zod-internals.mdc)
When adding issues, push well-formed payload.issues entries including code, expected (when applicable), input, inst, and optional path/message/continue
Files:
packages/zod/src/v4/core/schemas.ts
packages/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/zod-project-guide.mdc)
Write source code in TypeScript (TypeScript-first codebase)
Files:
packages/zod/src/v4/core/schemas.ts
packages/zod/src/v4/classic/tests/lazy.test.ts
packages/zod/**
📄 CodeRabbit inference engine (.cursor/rules/zod-project-guide.mdc)
Make core Zod library changes in the main package at packages/zod/
Files:
packages/zod/src/v4/core/schemas.ts
packages/zod/src/v4/classic/tests/lazy.test.ts
packages/zod/src/{v4/classic/tests,v4/core/tests,v3/tests}/**/*
📄 CodeRabbit inference engine (.cursor/rules/testing-guidelines.mdc)
Place all test files under packages/zod/src/v4/classic/tests, packages/zod/src/v4/core/tests, or packages/zod/src/v3/tests
Files:
packages/zod/src/v4/classic/tests/lazy.test.ts
packages/zod/src/{v4/classic/tests,v4/core/tests,v3/tests}/**/*.test.ts
📄 CodeRabbit inference engine (.cursor/rules/testing-guidelines.mdc)
packages/zod/src/{v4/classic/tests,v4/core/tests,v3/tests}/**/*.test.ts
: Test files must use the .test.ts extension (TypeScript), not JavaScript
Use import type for type-only imports in tests (e.g.,import type { ... }
)
Permanent, regression, API validation, edge-case coverage, and performance benchmark tests must be in the test suite (not play.ts)
Use Vitest as the framework in tests and import from it asimport { expect, test } from "vitest"
Import Zod in tests asimport * as z from "zod/v4"
Write tests with clear, descriptive names and cover both success and failure cases
Keep test suites concise while maintaining adequate coverage
Do not skip tests due to type issues; fix the types instead
Use descriptive file names like string.test.ts, object.test.ts, url-validation.test.ts and group related functionality together
Files:
packages/zod/src/v4/classic/tests/lazy.test.ts
packages/zod/src/v4/{classic,core}/tests/**/*.test.ts
📄 CodeRabbit inference engine (.cursor/rules/testing-workflow.mdc)
packages/zod/src/v4/{classic,core}/tests/**/*.test.ts
: Use Vitest for all testing (Vitest APIs in test files)
Place tests under packages/zod/src/v4/classic/tests/ or packages/zod/src/v4/core/tests/
Name test files with the *.test.ts suffix
Use test() for individual test cases
Use describe() for test groups
Use expect() for assertions
Files:
packages/zod/src/v4/classic/tests/lazy.test.ts
packages/**/*.test.ts
📄 CodeRabbit inference engine (.cursor/rules/zod-project-guide.mdc)
Use Vitest for tests and place test cases in .test.ts files
Files:
packages/zod/src/v4/classic/tests/lazy.test.ts
🧠 Learnings (2)
📚 Learning: 2025-10-21T17:27:32.477Z
Learnt from: CR
PR: colinhacks/zod#0
File: .cursor/rules/zod-internals.mdc:0-0
Timestamp: 2025-10-21T17:27:32.477Z
Learning: Applies to packages/zod/src/v4/core/schemas.ts : Define computed internal properties using util.defineLazy() to avoid circular dependencies
Applied to files:
packages/zod/src/v4/core/schemas.ts
packages/zod/src/v4/classic/tests/lazy.test.ts
📚 Learning: 2025-10-21T17:27:32.477Z
Learnt from: CR
PR: colinhacks/zod#0
File: .cursor/rules/zod-internals.mdc:0-0
Timestamp: 2025-10-21T17:27:32.477Z
Learning: Applies to packages/zod/src/v4/core/schemas.ts : Wrapper schemas (e.g., $ZodOptional, $ZodNullable, $ZodReadonly) must pass through internal properties from their inner type using util.defineLazy for propValues, values, optin, and optout
Applied to files:
packages/zod/src/v4/core/schemas.ts
1000f00
to
5a1e716
Compare
- add a lazy test that parses a two-node cycle and checks the circular refs - memoize parses per object input, including pending async results, so repeated visits reuse the same outcome
5a1e716
to
567fc0d
Compare
No description provided.