-
-
Notifications
You must be signed in to change notification settings - Fork 19
validate computed field configuration on startup #653
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: dev
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughAdded a private ClientImpl method that validates every computed field in the schema has a corresponding Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Adds runtime validation to ensure all @computed fields declared in the schema have corresponding computedFields implementations configured when the ORM client starts.
Changes:
- Add startup-time validation in
ClientImplconstructor to verify computed field config completeness. - Add E2E tests covering missing computed field configuration scenarios.
- Update existing type-checking computed-field tests to include runtime
computedFieldsconfig required by the new validation.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/e2e/orm/client-api/computed-fields.test.ts | Adds new negative tests for missing computedFields config and updates type-checking cases to satisfy new startup validation. |
| packages/orm/src/client/client-impl.ts | Introduces validateComputedFieldsConfig() and invokes it during client construction to fail fast on misconfiguration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ffee7d2 to
feaecb9
Compare
feaecb9 to
1212adc
Compare
Note on the changes to the existing tests:
The type-checking tests contain duplicate configurations:
Outer config (required by createTestClient): Needed to instantiate the actual client. Without it, the new runtime validation throws an error before the test can run.
Inner config (in extraSourceFiles): Example code that demonstrates proper usage and ensures the generated TypeScript file compiles with correct type hints.
This duplication is a consequence of using createTestClient for type-checking. The outer config satisfies the runtime validation, while the inner config serves as realistic documentation of how users should structure their code.
Let me know if you would prefer adding a helper to side-step createTestClient for these cases.
Summary by CodeRabbit
Bug Fixes
Tests