feat(zod): introduce an option to control depth of the built zod schemas#2392
Merged
feat(zod): introduce an option to control depth of the built zod schemas#2392
Conversation
Contributor
|
Claude finished @ymc9's task —— View job PR Review: Zod Schema Depth ControlReviewing PR #2392 that introduces an option to control depth of built Zod schemas. Todo List
Review SummaryThis PR introduces a valuable feature to control the depth of relation nesting in Zod schemas, which helps prevent infinite recursion and performance issues with deeply nested schemas. The implementation is well-designed and comprehensive. Code Quality & Best Practices ✅Strengths:
Minor Suggestions:
Implementation Analysis ✅Depth Logic Implementation:
Integration Points:
Performance Considerations ✅Positive Impact:
Cache Efficiency:
Security Considerations ✅Input Validation:
Test Coverage ✅Comprehensive Testing:
Testing Quality:
Potential Issues 🔍Minor Concerns:
Documentation Needs:
Recommendations
Overall AssessmentThis is a high-quality implementation that adds significant value:
The feature is ready for merge with only minor documentation suggestions. |
Contributor
📝 WalkthroughWalkthroughAdds a relation-depth option ( Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/orm/src/client/zod/factory.ts (2)
356-356: UsenextDepth/depthToOptionshelpers for consistency.Line 356 manually computes
depth !== undefined ? { depth: depth - 1 } : undefined, duplicating the logic ofthis.depthToOptions(this.nextDepth(depth))used elsewhere (e.g., lines 915, 953–954).♻️ Suggested change
- const nextOpts = depth !== undefined ? { depth: depth - 1 } : undefined; + const nextOpts = this.depthToOptions(this.nextDepth(depth));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/orm/src/client/zod/factory.ts` at line 356, Replace the manual depth computation at the assignment to nextOpts with the existing helpers: call this.nextDepth(depth) and pass its result into this.depthToOptions(...) instead of computing { depth: depth - 1 } inline; update the assignment to use this.depthToOptions(this.nextDepth(depth)) so it matches the pattern used in other places (see methods nextDepth and depthToOptions and the variable nextOpts).
133-143: Validate non-negative depth to prevent confusing behavior.If a caller passes a negative
depth(e.g.,{ depth: -1 }),shouldIncludeRelationsreturnsfalseandnextDepthdecrements further to -2, -3, etc. While this doesn't crash, it's unexpected API behavior—users likely expect depth ≥ 0 or undefined.🛡️ Suggested defensive check
private shouldIncludeRelations(depth?: number): boolean { return depth === undefined || depth > 0; } private nextDepth(depth?: number): number | undefined { return depth !== undefined ? depth - 1 : undefined; } private depthToOptions(depth?: number): CreateSchemaOptions | undefined { return depth !== undefined ? { depth } : undefined; }Add validation at the public API boundary (e.g., in each
make*Schemamethod) or indepthToOptions:+private validateDepth(depth?: number): void { + if (depth !== undefined && depth < 0) { + throw new Error(`"depth" must be a non-negative integer, received: ${depth}`); + } +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/orm/src/client/zod/factory.ts` around lines 133 - 143, The code currently allows negative depth values to propagate through shouldIncludeRelations, nextDepth, and depthToOptions causing unexpected behavior; add a defensive validation that rejects negative depths at the public API boundary (each make*Schema method) or centrally in depthToOptions: if depth is provided and depth < 0, throw a RangeError (with a clear message like "depth must be >= 0") so negative depths are rejected before shouldIncludeRelations or nextDepth run. Reference the methods shouldIncludeRelations, nextDepth, depthToOptions and the public make*Schema entry points when adding the check.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/zod/src/index.ts`:
- Line 1: The export rename removed the old exported symbol
createModelSchemaFactory which may break external consumers; restore a
deprecated re-export by re-exporting createSchemaFactory under the old name
createModelSchemaFactory (e.g., export const createModelSchemaFactory =
createSchemaFactory) and mark it deprecated in comments/TS docs so callers can
migrate while keeping the new createSchemaFactory as the canonical export;
update packages/zod/src/index.ts to add this alias export alongside the existing
export.
---
Nitpick comments:
In `@packages/orm/src/client/zod/factory.ts`:
- Line 356: Replace the manual depth computation at the assignment to nextOpts
with the existing helpers: call this.nextDepth(depth) and pass its result into
this.depthToOptions(...) instead of computing { depth: depth - 1 } inline;
update the assignment to use this.depthToOptions(this.nextDepth(depth)) so it
matches the pattern used in other places (see methods nextDepth and
depthToOptions and the variable nextOpts).
- Around line 133-143: The code currently allows negative depth values to
propagate through shouldIncludeRelations, nextDepth, and depthToOptions causing
unexpected behavior; add a defensive validation that rejects negative depths at
the public API boundary (each make*Schema method) or centrally in
depthToOptions: if depth is provided and depth < 0, throw a RangeError (with a
clear message like "depth must be >= 0") so negative depths are rejected before
shouldIncludeRelations or nextDepth run. Reference the methods
shouldIncludeRelations, nextDepth, depthToOptions and the public make*Schema
entry points when adding the check.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/orm/src/client/zod/factory.ts (2)
1888-1921:_optionsis accepted but intentionally unused inmakeProcedureParamSchema.The leading-underscore convention correctly signals an intentionally unused parameter. However, since procedures use scalar/enum/typedef types with no relation navigation, it's worth a brief inline comment to avoid future confusion about why it exists.
📝 Suggested inline note
`@cache`() makeProcedureParamSchema( param: { type: string; array?: boolean; optional?: boolean }, - _options?: CreateSchemaOptions, + _options?: CreateSchemaOptions, // reserved for API consistency; procedures have no relation nesting ): ZodType {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/orm/src/client/zod/factory.ts` around lines 1888 - 1921, Add a brief inline comment inside the makeProcedureParamSchema method explaining that the _options parameter is intentionally unused for procedure params because procedures only accept scalar/enum/typedef types (no relation navigation) so no schema-building options are needed; reference the _options parameter and the makeProcedureParamSchema function name so reviewers understand why it exists and won't be removed.
90-98: LGTM — consider adding a note on depth-0 semantics.The type and JSDoc are clear. One small addition that would help consumers: note that
relationDepth: 0means no relation fields are emitted (depth 0 = current model only, no nesting), whileundefinedmeans unlimited depth.📝 Suggested JSDoc clarification
export type CreateSchemaOptions = { /** - * Controls the depth of relation nesting in the generated schema. Default is unlimited. + * Controls the depth of relation nesting in the generated schema. + * - `undefined` (default): unlimited depth. + * - `0`: no relation fields are emitted. + * - `n > 0`: relation fields up to n levels deep are emitted. */ relationDepth?: number; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/orm/src/client/zod/factory.ts` around lines 90 - 98, Update the JSDoc for CreateSchemaOptions.relationDepth to explicitly state the semantics for 0 vs undefined: document that relationDepth: 0 emits no relation fields (only the current model), while leaving relationDepth undefined means unlimited nesting; modify the comment above the relationDepth property in the CreateSchemaOptions type to include these clarifying sentences so consumers understand depth-0 behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/orm/src/client/zod/factory.ts`:
- Around line 1888-1921: Add a brief inline comment inside the
makeProcedureParamSchema method explaining that the _options parameter is
intentionally unused for procedure params because procedures only accept
scalar/enum/typedef types (no relation navigation) so no schema-building options
are needed; reference the _options parameter and the makeProcedureParamSchema
function name so reviewers understand why it exists and won't be removed.
- Around line 90-98: Update the JSDoc for CreateSchemaOptions.relationDepth to
explicitly state the semantics for 0 vs undefined: document that relationDepth:
0 emits no relation fields (only the current model), while leaving relationDepth
undefined means unlimited nesting; modify the comment above the relationDepth
property in the CreateSchemaOptions type to include these clarifying sentences
so consumers understand depth-0 behavior.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a relationDepth option to control the depth of relation nesting in generated Zod schemas for ORM input validation. This allows users to limit the complexity of generated schemas and prevent deeply nested relation queries.
Changes:
- Added
CreateSchemaOptionstype withrelationDepthproperty to control schema generation depth - Renamed public export from
createModelSchemaFactorytocreateSchemaFactory(breaking change) - Added comprehensive test coverage for depth-based schema generation across all CRUD operations
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/zod/src/index.ts | Renamed export from createModelSchemaFactory to createSchemaFactory (breaking change) |
| packages/zod/test/factory.test.ts | Updated import to use new createSchemaFactory name |
| packages/orm/src/client/zod/factory.ts | Added CreateSchemaOptions type and threading of depth option through all schema factory methods |
| tests/e2e/orm/client-api/zod.test.ts | Added comprehensive test suite for depth-based schema generation (317 new lines) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
|
Claude finished @ymc9's task —— View job PR Review: Zod Schema Depth ControlReviewing PR #2392 that introduces an option to control depth of built Zod schemas. Todo List
Review SummaryThis PR introduces a valuable feature to control the depth of relation nesting in Zod schemas, which helps prevent infinite recursion and performance issues with deeply nested schemas. The implementation is well-designed and comprehensive. Code Quality & Best Practices ✅Strengths:
Minor Suggestions:
Implementation Analysis ✅Depth Logic Implementation:
Integration Points:
Performance Considerations ✅Positive Impact:
Cache Efficiency:
Security Considerations ✅Input Validation:
Test Coverage ✅Comprehensive Testing:
Testing Quality:
Breaking Changes
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
tests/e2e/orm/client-api/zod.test.ts (2)
1355-1368: Document the positionalbooleanparameters inmakeWhereSchemacalls.
makeWhereSchema('User', false, false, false, { ... })passes four undocumentedfalseflags; readers can't tell what they control without consulting the implementation. A brief inline comment on each call site would make the tests self-documenting.✏️ Example
-const s = client.$zod.makeWhereSchema('User', false, false, false, { relationDepth: 0 }); +// args: (model, isUniqueWhere, isNested, isOrFilter, options) — adjust names to match actual signature +const s = client.$zod.makeWhereSchema('User', false, false, false, { relationDepth: 0 });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/e2e/orm/client-api/zod.test.ts` around lines 1355 - 1368, Calls to client.$zod.makeWhereSchema('User', false, false, false, { relationDepth: 0 }) use four positional booleans that are unclear; update each test call to include short inline comments next to each boolean explaining what they represent (e.g., /* includeRelations */, /* includeJson */, etc. — use the actual parameter meanings from makeWhereSchema signature) so readers can understand the flags without opening the implementation; ensure you add similar comments for other usages like makeWhereSchema('Post', false, false, false, { relationDepth: 1 }) and keep the comments concise.
1066-1375: Depth tests are missing formakeUpdateManySchema,makeUpdateManyAndReturnSchema, andmakeCreateManyAndReturnSchema.All three accept
where/data/selectargs and are covered in the non-depth CRUD section, but they have no corresponding entries in theCreateSchemaOptions depthdescribe block. At minimum, arelationDepth: 0smoke test (analogous to themakeDeleteManySchemaormakeCountSchemadepth tests above) would close the gap.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/e2e/orm/client-api/zod.test.ts` around lines 1066 - 1375, Tests for depth behavior are missing for makeUpdateManySchema, makeUpdateManyAndReturnSchema, and makeCreateManyAndReturnSchema; add relationDepth: 0 smoke tests inside the existing "CreateSchemaOptions depth" describe block mirroring the pattern used for makeDeleteManySchema/makeCountSchema: create a schema via client.$zod.makeUpdateManySchema('User', { relationDepth: 0 }) (and likewise for makeUpdateManyAndReturnSchema and makeCreateManyAndReturnSchema) and assert that scalar-only inputs (e.g., where: { email: 'u@test.com' } or data: [{ email: 'u@test.com' }] / select: { posts: true } as applicable) succeed or fail appropriately—specifically ensure scalar where/data/select pass and relation filters (e.g., where: { posts: { some: { published: true } } }) or relation selects are rejected with .safeParse(...).success expectations matching the existing depth tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/e2e/orm/client-api/zod.test.ts`:
- Around line 1248-1253: The test for client.$zod.makeCreateManySchema('User', {
relationDepth: 0 }) currently only asserts acceptance of scalar-only data but
lacks a negative assertion or explanatory note; update the test to either assert
that nested relation input is rejected (e.g., safeParse({ data: [{ email:
'u@test.com', posts: { create: [...] } }] }).success is false) to prove the
depth gating works, or add a concise comment next to the makeCreateManySchema
invocation explaining that createMany does not support nested relation creates
in Prisma and therefore relationDepth: 0 is effectively a no-op for createMany;
reference makeCreateManySchema and client.$zod in the change so reviewers can
locate the test easily.
- Around line 1360-1365: The test currently checks that scalar predicates inside
AND/OR/NOT succeed at relationDepth: 0 but doesn't assert that relation filters
inside those logical operators are rejected; update the 'relationDepth: 0 still
allows logical operators' case (using client.$zod.makeWhereSchema('User', false,
false, false, { relationDepth: 0 })) to add negative assertions that logical
combinators containing a relation filter (e.g., AND/OR array elements or NOT
value that include a relation field like posts:{ some: {...} } or similar
relation-filter shape used elsewhere) return .safeParse(...).success === false
so the depth restriction is enforced through logical operators.
---
Nitpick comments:
In `@tests/e2e/orm/client-api/zod.test.ts`:
- Around line 1355-1368: Calls to client.$zod.makeWhereSchema('User', false,
false, false, { relationDepth: 0 }) use four positional booleans that are
unclear; update each test call to include short inline comments next to each
boolean explaining what they represent (e.g., /* includeRelations */, /*
includeJson */, etc. — use the actual parameter meanings from makeWhereSchema
signature) so readers can understand the flags without opening the
implementation; ensure you add similar comments for other usages like
makeWhereSchema('Post', false, false, false, { relationDepth: 1 }) and keep the
comments concise.
- Around line 1066-1375: Tests for depth behavior are missing for
makeUpdateManySchema, makeUpdateManyAndReturnSchema, and
makeCreateManyAndReturnSchema; add relationDepth: 0 smoke tests inside the
existing "CreateSchemaOptions depth" describe block mirroring the pattern used
for makeDeleteManySchema/makeCountSchema: create a schema via
client.$zod.makeUpdateManySchema('User', { relationDepth: 0 }) (and likewise for
makeUpdateManyAndReturnSchema and makeCreateManyAndReturnSchema) and assert that
scalar-only inputs (e.g., where: { email: 'u@test.com' } or data: [{ email:
'u@test.com' }] / select: { posts: true } as applicable) succeed or fail
appropriately—specifically ensure scalar where/data/select pass and relation
filters (e.g., where: { posts: { some: { published: true } } }) or relation
selects are rejected with .safeParse(...).success expectations matching the
existing depth tests.
Contributor
|
Claude finished @ymc9's task —— View job PR Review: Zod Schema Depth ControlReviewing PR #2392 that introduces an option to control depth of built Zod schemas. Todo List
Review SummaryThis PR introduces a valuable feature to control the depth of relation nesting in Zod schemas, which helps prevent infinite recursion and performance issues with deeply nested schemas. The implementation is well-designed and comprehensive. Code Quality & Best Practices ✅Strengths:
Minor Suggestions:
Implementation Analysis ✅Depth Logic Implementation:
Integration Points:
Performance Considerations ✅Positive Impact:
Cache Efficiency:
Security Considerations ✅Input Validation:
Test Coverage ✅Comprehensive Testing:
Testing Quality:
Breaking Changes
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@samples/nuxt/package.json`:
- Line 9: The package.json "build" script currently runs "pnpm generate && nuxt
typecheck", which only type-checks and does not produce a bundled artifact;
update the "build" script so it performs an actual build (e.g., use "nuxt build"
or "nuxt build && nuxt generate" instead of "nuxt typecheck"), or alternatively
add a separate "typecheck" script and keep "build" for bundling—change the
"build" entry in package.json accordingly to ensure a deployable output is
created.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Breaking Changes
Tests
Chores