feat(zod): add ORM-style select/include/omit options to makeModelSchema#2503
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughmakeModelSchema gains an options-aware overload supporting ORM-style 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/factory.ts`:
- Around line 98-103: The current logic only skips model-level @@validate when
rawOptions.select is present, but rawOptions.omit can also remove fields
referenced by validations and cause false negatives or impossible schemas;
update the branch that sets withValidation to consider rawOptions.omit as well:
inspect which fields the model-level validations reference (from
modelDef.attributes or the structure holding @@validate expressions) and only
call addCustomValidation(shape, modelDef.attributes) when neither select nor
omit remove any of those referenced fields; otherwise use the plain shape.
Ensure you reference rawOptions.select, rawOptions.omit, withValidation,
addCustomValidation, and modelDef.attributes when locating and changing the
code.
In `@packages/zod/src/types.ts`:
- Around line 272-295: BuildSelectShape currently keeps keys when a select entry
widens from literal true to boolean but SelectEntryToZod only handles literal
true or object, causing widened booleans to resolve to never; update
SelectEntryToZod to treat any boolean (Value extends boolean) the same as true
(i.e., return GetModelFieldsShape<Schema, Model>[FieldInShape<Schema, Model,
Field>]) before falling back to the object/never branches, and add a unit test
that uses a boolean variable (e.g., const pickEmail: boolean = true;
factory.makeModelSchema('User', { select: { email: pickEmail } })) to assert the
produced type exposes the field shape instead of never; reference types:
SelectEntryToZod, BuildSelectShape, GetModelFieldsShape, FieldInShape.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3fb3fc2e-91f0-43fd-b5ab-b10630375584
📒 Files selected for processing (4)
packages/zod/src/factory.tspackages/zod/src/index.tspackages/zod/src/types.tspackages/zod/test/factory.test.ts
ymc9
left a comment
There was a problem hiding this comment.
Hi @marcsigmund , thanks for making this nice PR. Overall it's looking very good. I've left a few comments there.
…chema Add support for field selection and relation inclusion in Zod schema generation through new options parameter. The `makeModelSchema` method now accepts optional `select`, `include`, and `omit` options to control which fields appear in the generated schema. Key changes: - Add overloaded signatures to `makeModelSchema` with type-safe options - Implement `buildFieldsWithOptions` to handle field filtering and relation inclusion - Add `GetModelSchemaShapeWithOptions` type to compute resulting shape based on options - Skip model-level @@Validate on select path to avoid false negatives when referenced fields are not part of the selection - Add comprehensive type utilities for field selection and relation handling This enables more flexible schema generation for partial model validation and nested relation schemas while maintaining full type safety.
- Skip @@Validate rules per-rule based on which fields are present in the resulting shape, instead of skipping all rules when any omit/select is used. Rules whose referenced fields are all present still apply. - Handle widened boolean (not just literal true) in SelectEntryToZod to prevent field types resolving to never - Validate options at runtime with a recursive Zod schema, enforcing that select/include and select/omit cannot be used together - Rename 'Prisma-style' to 'ORM-style' in comments and docs"
27e4592 to
64cf273
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/factory.ts`:
- Around line 202-257: buildFieldsWithOptions currently silently ignores unknown
field names and type-mismatched option values; instead validate option keys and
value shapes and throw SchemaFactoryError when invalid. In
buildFieldsWithOptions (use modelDef and the select/include/omit branches) check
each key from select/include/omit exists in modelDef.fields—if not, throw
SchemaFactoryError naming the bad key and model; for select: if
fieldDef.relation is true but select[key] is strictly true (no nested options)
accept, but if select[key] is an object for a scalar field (fieldDef.relation
falsy) throw; for include: if include[key] is provided for a non-relation scalar
field, throw; for omit: ensure each omit key exists and is a boolean (or
truthy/falsey as expected) otherwise throw. Use existing helpers
makeRelationFieldSchema, makeScalarFieldSchema, applyCardinality and the
SchemaFactoryError class to surface errors instead of silently skipping.
In `@packages/zod/src/utils.ts`:
- Around line 293-298: The partial-shape guard currently only checks flat field
names against presentFields using collectFieldRefs and therefore allows member
expressions like author.email to pass when only nested selections exist; update
the logic in the block around collectFieldRefs (and similarly the code handling
lines ~315-337) to either (1) make collectFieldRefs return full nested paths
(e.g., "author.email") and compare those paths against presentFields, or (2)
conservatively skip member-based rules when a relation is projected with
sub-options (i.e., detect when presentFields contains a nested selection object
for a receiver like "author" and treat any member refs on that receiver as not
present); implement the chosen approach consistently for the other guarded
section and add a regression test demonstrating that a rule like
@@validate(author.email != null) is not attached when only select: { author: {
select: { id: true } } } is provided.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0b2e9915-baeb-4fd1-9708-fc5d5b836d85
📒 Files selected for processing (5)
packages/zod/src/factory.tspackages/zod/src/index.tspackages/zod/src/types.tspackages/zod/src/utils.tspackages/zod/test/factory.test.ts
✅ Files skipped from review due to trivial changes (1)
- packages/zod/src/index.ts
There was a problem hiding this comment.
🧹 Nitpick comments (3)
packages/zod/src/factory.ts (1)
296-329: Minor inconsistency:buildPresentShapesilently skips unknown fields.In
buildPresentShape, whenfieldDefis not found (lines 304-305 and 320-321), the code silently continues. While this is safe becausebuildFieldsWithOptionsalready validates and throws for unknown fields, maintaining the same validation pattern would make the code more defensive.However, this is a minor concern since the two methods are always called together and
buildFieldsWithOptionsruns first (line 128 before line 137).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/zod/src/factory.ts` around lines 296 - 329, buildPresentShape currently ignores unknown fields when looking up field definitions (in the branches handling select and include); make it defensive by throwing a clear error instead of silently continuing: when const fieldDef = modelDef.fields[key] yields falsy, throw an informative Error referencing the model and field (same style/message used in buildFieldsWithOptions) so callers like buildFieldsWithOptions remain consistent; update both occurrences inside buildPresentShape to perform this validation.packages/zod/test/factory.test.ts (1)
1204-1240: Consider adding a test for nested member expression in@@validate.The tests verify that
@@validate(age >= 18)is skipped whenageis omitted, but there's no test for nested member expressions like@@validate(author.email != null)being skipped when onlyselect: { author: { select: { id: true } } }is used. This was a concern raised in past reviews.💡 Suggested test case
If the schema supports such a validation rule, consider adding:
it('@@validate is skipped when nested relation field ref is not fully selected', () => { // Assuming a model with @@validate(author.email != null) or similar const schema = factory.makeModelSchema('Post', { select: { id: true, author: { select: { id: true } }, // email not selected }, }); // author.email is not in the shape, so the validation should be skipped expect(schema.safeParse({ id: 'p1', author: { id: 'u1' } }).success).toBe(true); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/zod/test/factory.test.ts` around lines 1204 - 1240, Add a test to cover nested member-expression validation skipping: use factory.makeModelSchema('Post', { select: { id: true, author: { select: { id: true } } } }) to build a schema for a model that has an @@validate referencing a nested field (e.g., @@validate(author.email != null)), and assert that schema.safeParse({ id: 'p1', author: { id: 'u1' } }).success is true to ensure the @@validate rule is skipped when the nested relation field (author.email) is not selected; place this alongside the other runtime validation tests so it exercises the same skipping logic for nested member expressions.packages/zod/src/utils.ts (1)
389-417: Consider whethercollectFieldRefsis still needed.
collectFieldRefsis exported but unused internally. If it's only exposed for external use without active internal calls, mark it as@internalvia JSDoc or remove it to reduce the public API surface.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/zod/src/utils.ts` around lines 389 - 417, The exported function collectFieldRefs appears unused internally; either reduce API surface by removing it or mark it internal — search for usages of collectFieldRefs outside this module and if none exist, delete the export (or the whole function) and remove any re-exports, otherwise add a JSDoc /** `@internal` */ immediately above the collectFieldRefs declaration to hide it from public docs/typing outputs and ensure any barrel exports (index files) are not exposing it publicly.
🤖 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/zod/src/factory.ts`:
- Around line 296-329: buildPresentShape currently ignores unknown fields when
looking up field definitions (in the branches handling select and include); make
it defensive by throwing a clear error instead of silently continuing: when
const fieldDef = modelDef.fields[key] yields falsy, throw an informative Error
referencing the model and field (same style/message used in
buildFieldsWithOptions) so callers like buildFieldsWithOptions remain
consistent; update both occurrences inside buildPresentShape to perform this
validation.
In `@packages/zod/src/utils.ts`:
- Around line 389-417: The exported function collectFieldRefs appears unused
internally; either reduce API surface by removing it or mark it internal —
search for usages of collectFieldRefs outside this module and if none exist,
delete the export (or the whole function) and remove any re-exports, otherwise
add a JSDoc /** `@internal` */ immediately above the collectFieldRefs declaration
to hide it from public docs/typing outputs and ensure any barrel exports (index
files) are not exposing it publicly.
In `@packages/zod/test/factory.test.ts`:
- Around line 1204-1240: Add a test to cover nested member-expression validation
skipping: use factory.makeModelSchema('Post', { select: { id: true, author: {
select: { id: true } } } }) to build a schema for a model that has an @@validate
referencing a nested field (e.g., @@validate(author.email != null)), and assert
that schema.safeParse({ id: 'p1', author: { id: 'u1' } }).success is true to
ensure the @@validate rule is skipped when the nested relation field
(author.email) is not selected; place this alongside the other runtime
validation tests so it exercises the same skipping logic for nested member
expressions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8eef3fb9-c715-4922-b81a-4f2f48dd544b
📒 Files selected for processing (3)
packages/zod/src/factory.tspackages/zod/src/utils.tspackages/zod/test/factory.test.ts
Add support for ORM-style field selection and relation inclusion in Zod schema generation. The
makeModelSchemamethod now accepts optionalselect,include, andomitoptions to control which fields appear in the generated schema.Key changes:
makeModelSchemawith type-safe optionsbuildFieldsWithOptionsfor field filtering and relation inclusionGetModelSchemaShapeWithOptionstype to compute the resulting shape based on options@@validaterules selectively – each rule only runs when all fields it references are present in the resulting shape, keeping the zmodel the single source of truth for validationExample:
Summary by CodeRabbit
New Features
Types
Tests