fix(orm): handle cyclic JSON typedef references in zod factory (#2654)#2655
Conversation
Cyclic or self-referencing JSON typedefs caused makeTypeDefSchema to recurse forever — the @cache() decorator only stores the result after the method returns, so a recursive call back into the same type still saw an empty cache. Wrap nested typedef references in z.lazy() so the inner lookup is deferred to validation time, by which point the outer build has populated the cache. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR fixes a stack overflow when creating data for JSON types with cyclic references by wrapping nested typedef field schemas in ChangesCyclic Typedef Schema Generation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
|
Claude finished @ymc9's task —— View job Code Review for PR #2655Todo List:
Review SummaryThis PR effectively solves the cyclic typedef reference issue by implementing a deferred evaluation pattern using Code Quality & Implementation ✅Factory.ts Changes ( The implementation is clean and follows the established pattern already used in let fieldSchema: ZodType = isTypeDef(this.schema, def.type)
? z.lazy(() => this.makeTypeDefSchema(def.type))
: this.makeScalarSchema(def.type);Strengths:
Test Coverage ✅Comprehensive test cases ( The test file covers both major cyclic scenarios:
Test quality highlights:
Performance Considerations ✅
Security Considerations ✅
Potential Issues & SuggestionsMinor Considerations:
Code Quality Observations:The fix is minimal, targeted, and follows established patterns. The implementation correctly identifies that the root cause was the Overall AssessmentExcellent fix that:
This PR is ready for merge. The implementation is clean, well-tested, and follows TypeScript/Zod best practices. |
Summary
makeTypeDefSchemapreviously recursed forever when JSON typedefs referenced each other (or themselves), throwingRangeError: Maximum call stack size exceeded. The@cache()decorator only populates the cache after the method returns, so the recursive call still saw an empty cache.z.lazy(() => …)so the inner lookup defers to validation time, by which point the outer build has finished and cached its result. Same pattern already used bymakeJsonValueSchema.tests/regression/test/issue-2654.test.tscovering cyclic (A↔B) and self-referencing (Tree { children Tree[]? }) typedefs.Fixes #2654
Test plan
tests/regression/test/issue-2654.test.tspassestests/regression/test/issue-{493,558,586}.test.ts(existing JSON typedef coverage) still passtests/e2e/orm/client-api/typed-json-fields.test.tsstill passes🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests