-
-
Notifications
You must be signed in to change notification settings - Fork 17
feat(orm): implement client API extensions, refactor query args extension #603
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
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughReworks plugin typings and runtime shape: introduces AnyPlugin, expands RuntimePlugin generics (queryArgs, client members), adds per-operation core operation types, threads ExtClientMembers through client contracts, updates validator to resolve per-operation plugin schemas, adjusts client proxy to expose plugin-provided $-members, and updates tests. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
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
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 Completed ✅I've thoroughly reviewed this PR that implements client API extensions and refactors query args extension. The code demonstrates good architecture, comprehensive testing, and proper TypeScript type safety. Summary of ChangesThis PR adds two key plugin extension capabilities:
Code Quality Assessment✅ Strengths1. Clean Architecture Design
2. Type Safety & Developer Experience
3. Comprehensive Validation
4. Robust Test Coverage
|
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.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@packages/orm/src/client/plugin.ts`:
- Around line 76-78: The plugin `queryArgs` type currently allows any
ZodType<ExtQueryArgs[K]>, but the validator `getPluginExtQueryArgsSchema`
expects a ZodObject at runtime, causing a mismatch; fix by narrowing the
exported type in packages/orm/src/client/plugin.ts from ZodType<ExtQueryArgs[K]>
to ZodObject<...> (constraining shape to an object schema) so only ZodObject
instances can be passed to `queryArgs`, or instead update the validator function
`getPluginExtQueryArgsSchema` in validator/index.ts to accept and correctly
handle non-object Zod types (e.g., ZodUnion/ZodIntersection) by removing or
adjusting the invariant check; choose one approach and apply consistent
types/guards in both `queryArgs` and `getPluginExtQueryArgsSchema`.
In `@tests/e2e/orm/plugin-infra/client-members.test.ts`:
- Around line 55-59: The test uses async-await with synchronous Jest assertions
— remove the unnecessary awaits so assertions run synchronously; change
occurrences like await expect(extDb.$cacheStats.hits).toBe(10) and await
expect(extDb.$cacheStats.misses).toBe(5) (and similarly for
extDb.$cacheStats1.hits/misses) to plain expect(...) without await to use the
correct expect().toBe() pattern.
In `@tests/e2e/orm/plugin-infra/ext-query-args.test.ts`:
- Line 25: Remove the stray empty template literal token `` that was left in the
test file (orphaned ``;). Edit the test source to delete that artifact so the
file contains only valid statements; verify the surrounding test suite
(ext-query-args.test) still compiles and runs after removal.
🧹 Nitpick comments (1)
packages/orm/src/client/crud/validator/index.ts (1)
385-430: Well-structured per-operation schema resolution with cascading fallback.The helper correctly implements the precedence order: specific operation → grouped operations ($create/$read/$update/$delete) → $all fallback. The type guards using
.includes()with the const arrays properly narrow the operation type.One minor observation: the invariant at lines 425-428 will throw if a plugin provides a non-ZodObject value (e.g., a ZodUnion or ZodIntersection). This is appropriate for strict validation, but the error message could be more specific about which plugin caused the issue.
💡 Optional: Include plugin ID in error message for easier debugging
- invariant( - result === undefined || result instanceof z.ZodObject, - 'Plugin extended query args schema must be a Zod object', - ); + invariant( + result === undefined || result instanceof z.ZodObject, + `Plugin "${plugin.id}" extended query args schema must be a Zod object`, + );
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
This pull request refactors the ORM plugin infrastructure by implementing client API extensions and improving the query args extension system.
Changes:
- Refactored plugin query args API from callback-based
extQueryArgs.getValidationSchema()to declarativequeryArgsobject with grouped operations ($create,$read,$update,$delete,$all) - Added client member extension capability allowing plugins to add methods and properties to the client
- Introduced new operation group constants (
CoreCreateOperations,CoreUpdateOperations,CoreDeleteOperations) for better query args organization
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
tests/e2e/orm/plugin-infra/on-query-hooks.test.ts |
Removed type parameter from definePlugin call |
tests/e2e/orm/plugin-infra/ext-query-args.test.ts |
Updated tests to use new declarative queryArgs API with grouped operations |
tests/e2e/orm/plugin-infra/client-members.test.ts |
Added comprehensive test coverage for new client member extension feature |
packages/plugins/policy/src/plugin.ts |
Updated PolicyPlugin to implement new RuntimePlugin interface with three type parameters |
packages/orm/src/client/plugin.ts |
Refactored plugin interface with new queryArgs field and ExtClientMembersBase type, updated definePlugin signature |
packages/orm/src/client/options.ts |
Replaced RuntimePlugin<any, any> with AnyPlugin type alias |
packages/orm/src/client/index.ts |
Exported new operation group types |
packages/orm/src/client/crud/validator/index.ts |
Implemented validation logic for grouped query args with precedence handling |
packages/orm/src/client/crud/operations/base.ts |
Added CoreCreateOperations, CoreUpdateOperations, and CoreDeleteOperations constants |
packages/orm/src/client/contract.ts |
Updated all client type signatures to include ExtClientMembers generic parameter |
packages/orm/src/client/client-impl.ts |
Enhanced client proxy to support plugin-provided members with reverse-order lookup |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Claude finished @ymc9's task —— View job PR Code Review Completed ✅I've analyzed this PR that implements client API extensions and refactors query args extension. The implementation demonstrates solid architecture, comprehensive testing, and proper type safety. Review Tasks
SummaryThis PR adds two major plugin extension capabilities:
Code Quality Assessment✅ Strengths1. Clean Architecture
2. Excellent Type Safety
3. Robust Validation System
4. Comprehensive Testing
|
* Initial plan * fix: merge $create and $update schemas for upsert validation - Handle upsert operation specially to match TypeScript type behavior - When both $create and $update schemas exist, merge them for upsert - Add test case to verify the fix works correctly Co-authored-by: ymc9 <[email protected]> * fix: improve comment accuracy about Zod merge behavior Co-authored-by: ymc9 <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: ymc9 <[email protected]>
|
Claude finished @ymc9's task —— View job PR Code Review ✅I've thoroughly reviewed this PR that implements client API extensions and refactors query args extension. The code demonstrates solid architecture, comprehensive testing, and proper type safety. Review Tasks
SummaryThis PR adds two major plugin extension capabilities:
Code Quality Assessment✅ Strengths1. Clean Architecture
2. Excellent Type Safety
3. Robust Validation System
4. Comprehensive Testing
|
|
Claude finished @ymc9's task —— View job PR Code Review ✅I've thoroughly reviewed this PR that implements client API extensions and refactors query args extension. The code demonstrates solid architecture, comprehensive testing, and proper type safety. Review Tasks
SummaryThis PR adds two major plugin extension capabilities:
Code Quality Assessment✅ Strengths1. Clean Architecture
2. Excellent Type Safety
3. Robust Validation System
4. Comprehensive Testing
|
Summary by CodeRabbit
New Features
Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.