-
-
Notifications
You must be signed in to change notification settings - Fork 91
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
refactor(zod): clean up optionality of generated schema fields #1139
Conversation
WalkthroughWalkthroughThe recent update refines the schema generation logic in a comprehensive manner, focusing on enhancing the flexibility and accuracy of schema definitions for models, particularly concerning Prisma create and update operations. It introduces selective handling of relations, schema refinement functions, and the export of both refined and unrefined schemas, alongside specific enhancements for user model validation. Changes
Assessment against linked issues
The assessment indicates that while the PR introduces significant improvements to schema generation and model validation, it lacks clear evidence on addressing the specific objectives related to the optional field behavior in the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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.
Review Status
Actionable comments generated: 4
Configuration used: CodeRabbit UI
Files selected for processing (2)
- packages/schema/src/plugins/zod/generator.ts (6 hunks)
- tests/integration/tests/plugins/zod.test.ts (6 hunks)
Additional comments: 11
packages/schema/src/plugins/zod/generator.ts (10)
- 314-314: The introduction of
baseSchema
usingmakeFieldSchema
for scalar fields with optionality is a good approach. However, ensure thatmakeFieldSchema
properly handles all scalar types and their nuances, especially custom types or those requiring special validation.- 328-332: The handling of relation fields using
makeFieldSchema
without the optionality flag is appropriate. It's important to verify that relation fields are correctly identified and that their schemas are generated accurately, considering the potential complexity of nested relations.- 383-390: Exporting a schema with only scalar fields as
[Model]ScalarSchema
is a useful feature for scenarios requiring simplified validation. Ensure that this schema accurately reflects all scalar fields and their validation rules without including relations or foreign keys.- 398-399: Merging relation fields into the model schema and making them all optional is a logical step for flexibility. However, it's important to ensure that this does not lead to unintended consequences, such as bypassing necessary validations. Consider documenting this behavior clearly for users of the generated schemas.
Consider adding documentation or comments explaining the decision to make all relation fields optional in the merged schema.
- 405-412: Exporting a schema without refinement for extensibility is thoughtful, allowing users to extend the schema further if needed. It's essential to document this feature prominently to ensure users are aware of its existence and how to use it effectively.
Recommend adding more detailed documentation or examples on how to use the
[Model]WithoutRefineSchema
for further customizations.
- 416-422: The final model schema export, including all fields and validations, is the culmination of the schema generation process. Ensure that this schema accurately represents the model, including all scalar, foreign key, and relation fields, along with any applied refinements.
- 433-439: The schema used for validating Prisma create input is marked for internal use only. It's important to ensure that this schema is correctly generated and that it aligns with the intended use cases for Prisma create operations.
- 458-466: Similarly, the schema for validating Prisma update input should be thoroughly tested to ensure it correctly handles all potential update scenarios, including atomic operations on numeric fields.
- 471-517: The creation of separate schemas for create operations, including handling fields with defaults as optional, is a nuanced approach. Ensure that this logic correctly identifies fields with defaults and applies optionality as intended.
- 522-560: For update operations, making all fields optional is standard, but special attention should be paid to the handling of foreign key fields and the application of refinements. Verify that the update schema behaves as expected in various update scenarios.
tests/integration/tests/plugins/zod.test.ts (1)
- 48-50: Adding an optional
age
field to theUser
model and a validation rule for password length is a good practice for enhancing the model's robustness. Ensure that these additions are reflected in the corresponding schemas and that the tests adequately cover these new aspects.
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- packages/schema/src/plugins/zod/generator.ts (7 hunks)
Files skipped from review as they are similar to previous changes (1)
- packages/schema/src/plugins/zod/generator.ts
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- tests/integration/tests/regression/issue-886.test.ts (1 hunks)
Additional comments: 1
tests/integration/tests/regression/issue-886.test.ts (1)
- 16-20: The test case has been updated to include an explicit
id
field with a value of1
in the input object forModelSchema.parse()
, and the expectation for theid
property has been adjusted accordingly. This change effectively tests the handling of explicitid
values in the schema, which is relevant to the regression issue being addressed. Ensure that this test case aligns with the intended behavior of the schema regardingid
fields and default values.
Fixes #898
Summary by CodeRabbit