-
-
Notifications
You must be signed in to change notification settings - Fork 90
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
fix: preserve prisma client extensions's typing #1148
Conversation
WalkthroughWalkthroughThe recent changes involve enhancing the Changes
Assessment against linked issues
This assessment indicates that while the main objectives related to coding have been addressed, there is a lack of explicit mention of documentation updates and a complete verification through testing, which leaves some ambiguity regarding the thoroughness of the resolution. 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: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- packages/schema/src/plugins/enhancer/enhance/index.ts (1 hunks)
- tests/integration/tests/enhancements/with-delegate/enhanced-client.test.ts (3 hunks)
Additional comments: 3
packages/schema/src/plugins/enhancer/enhance/index.ts (1)
- 95-127: The implementation of function overloads for
enhance
effectively addresses the need for supporting both plain and extendedPrismaClient
instances. This flexibility is crucial for the polymorphism feature to work seamlessly with Prisma client extensions. However, there are a few points to consider for refinement:
Type Safety and Documentation: Ensure that the types used in the function signatures (
ExtArgs
,TypeMap
,TypeMapCb
, etc.) are well-documented. This will help developers understand the expected structure and usage of these types, especially when extending thePrismaClient
.Error Handling: Consider adding error handling within the
enhance
function to manage cases where the providedprisma
instance does not match any of the expected types. This could involve throwing a descriptive error or logging a warning to assist in debugging.Performance Considerations: Given that the
enhance
function is likely to be called frequently, evaluate the performance implications of the type checks and operations performed within the function. If there are any potential bottlenecks, consider optimizing these areas.Testing: Ensure comprehensive testing of the
enhance
function, covering various scenarios with both plain and extendedPrismaClient
instances. This includes testing with different combinations ofcontext
andoptions
arguments to verify that the function behaves as expected in all cases.Overall, the changes to the
enhance
function are a positive step towards resolving the issue with polymorphism's typing and Prisma client extensions. Addressing the above points will further strengthen the implementation and ensure a robust solution.tests/integration/tests/enhancements/with-delegate/enhanced-client.test.ts (2)
- 1080-1161: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1040-1095]
The test case for TypeScript compilation with a plain Prisma client is well-structured and demonstrates the intended use case effectively. However, consider adding assertions to verify the expected behavior of the operations performed within the
main
function. This would ensure that the enhancements applied to the Prisma client are functioning as expected.Consider adding assertions or checks after the database operations to verify the expected outcomes. This could involve checking the properties of the created entities or the results of queries to ensure they match expected values.
- 1097-1159: Similar to the previous comment, the test case for TypeScript compilation with an extended Prisma client is well-structured. It effectively demonstrates extending the Prisma client with custom methods. As with the plain Prisma client test case, adding assertions to verify the expected behavior would enhance the test's effectiveness.
Adding assertions or checks after database operations in the
main
function would help verify that the enhancements and custom methods applied to the Prisma client are working as intended. This could involve checking the properties of the created entities or the results of queries.
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/cli/plugins.test.ts (2 hunks)
Additional comments: 2
tests/integration/tests/cli/plugins.test.ts (2)
- 76-76: Please ensure that the project and its tests are fully compatible with
@prisma/client
version^5.0.0
. This may involve reviewing the release notes for breaking changes or new features that could impact the project.- 86-86: Please ensure that the development environment and build processes are fully compatible with
prisma
version^5.0.0
. Reviewing the release notes for potential breaking changes or new features that could affect the project is advisable.
Fixes #1140
Summary by CodeRabbit