-
-
Notifications
You must be signed in to change notification settings - Fork 104
merge dev to main (v2.12.1) #2026
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
📝 WalkthroughWalkthroughThis pull request makes a version bump in the JetBrains IDE build configuration, refines documentation for a foreign key mapping in a model metadata file, and simplifies the handling of nested foreign key logic in the default authentication handler. It also streamlines proxy creation by removing a redundant conditional check and updating function signatures to pass additional context. In addition, new integration and regression tests have been added to validate proxy extension context behavior and schema-related functionality. Changes
Sequence Diagram(s)sequenceDiagram
participant T as Test Suite
participant DB as Enhanced Database Client
participant MP as makeProxy & createHandlerProxy
participant P as Proxy Handler
T->>DB: Call enhanced method (e.g., createWithCounter)
DB->>MP: Invoke proxy creation with $extends result and pass dbOrTx
MP->>P: Create proxy handler with additional context
P-->>DB: Provide context (e.g., $parent) when required
DB-->>T: Return processed result (e.g., created record with counter updated)
Possibly related PRs
✨ Finishing Touches
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration 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.
Actionable comments posted: 0
🧹 Nitpick comments (5)
tests/regression/tests/issue-2014.test.ts (2)
27-28
: Consider adding a comment explaining the purpose of the empty extension.This code creates an extended database with an empty configuration object. Since this is a regression test, it would be helpful to add a comment explaining why an empty extension is being used and what specific regression it's testing.
const db = enhance(user); -const extendedDb = db.$extends({}); +// Test that nested object creation works correctly even with an empty extension +const extendedDb = db.$extends({});
30-37
: Hardcoded ID in the assertion could make the test brittle.The test expects the newly created user to have
id: 2
, which assumes specific database state. Consider making this more resilient by using a dynamic assertion that doesn't depend on specific ID values.await expect( extendedDb.user.create({ data: {}, }) -).resolves.toEqual({ - id: 2, - tenantId: tenant.id, +).resolves.toMatchObject({ + tenantId: tenant.id, });packages/runtime/src/enhancements/node/proxy.ts (3)
270-270
: Simplification of the proxy creation logic.The code now always calls
makeProxy
on the result of$extends
, which simplifies the proxy creation process. Previously, there might have been a conditional check forresult[PRISMA_PROXY_ENHANCER]
that has been removed.Consider adding a brief comment explaining the change in behavior to help future maintainers understand the rationale behind always recreating the proxy for extended clients.
301-301
: Improved function signature with explicit parameter.The
dbOrTx
parameter is now explicitly defined in the function signature, which helps clarify the intent and usage of this parameter.Consider using a more specific type than
any
for thedbOrTx
parameter to improve type safety. For example:-dbOrTx: any, +dbOrTx: DbClientContract | Transaction,This would require importing the appropriate types, but would enhance type safety and code readability.
306-308
: Added access to parent database or transaction context.This change introduces the ability to access the parent database or transaction context through the
$parent
property, which enhances the proxy's capabilities for extension methods that need to reference the parent context.Consider adding JSDoc comments to document the purpose and usage of the
$parent
property, which would help users understand how to utilize this feature correctly:+/** + * Returns the parent database or transaction context. + * This is useful for extension methods that need to reference higher-level operations. + */ if (propKey === '$parent') { return dbOrTx; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (13)
package.json
is excluded by!**/*.json
packages/ide/jetbrains/package.json
is excluded by!**/*.json
packages/language/package.json
is excluded by!**/*.json
packages/misc/redwood/package.json
is excluded by!**/*.json
packages/plugins/openapi/package.json
is excluded by!**/*.json
packages/plugins/swr/package.json
is excluded by!**/*.json
packages/plugins/tanstack-query/package.json
is excluded by!**/*.json
packages/plugins/trpc/package.json
is excluded by!**/*.json
packages/runtime/package.json
is excluded by!**/*.json
packages/schema/package.json
is excluded by!**/*.json
packages/sdk/package.json
is excluded by!**/*.json
packages/server/package.json
is excluded by!**/*.json
packages/testtools/package.json
is excluded by!**/*.json
📒 Files selected for processing (7)
packages/ide/jetbrains/build.gradle.kts
(1 hunks)packages/runtime/src/cross/model-meta.ts
(1 hunks)packages/runtime/src/enhancements/node/default-auth.ts
(1 hunks)packages/runtime/src/enhancements/node/proxy.ts
(3 hunks)tests/integration/tests/enhancements/proxy/extension-context.test.ts
(1 hunks)tests/regression/tests/issue-2014.test.ts
(1 hunks)tests/regression/tests/issue-2019.test.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- packages/ide/jetbrains/build.gradle.kts
- packages/runtime/src/cross/model-meta.ts
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: OSSAR-Scan
- GitHub Check: build-test (20.x)
- GitHub Check: dependency-review
- GitHub Check: build-test (20.x)
- GitHub Check: build-test (20.x)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (12)
tests/regression/tests/issue-2014.test.ts (2)
5-22
: Well-structured schema definition with clear relationships.The schema is well-defined with a clear one-to-many relationship between
Tenant
andUser
. The@default(auth().tenantId)
attribute on thetenantId
field ensures that the current tenant's ID is automatically applied.
24-25
: The data creation steps are clear and correctly implemented.The tenant and user creation steps follow a logical flow, correctly establishing the parent-child relationship by setting the
tenantId
explicitly.tests/regression/tests/issue-2019.test.ts (3)
5-64
: Comprehensive schema definition with proper model relationships.The schema is well-structured with clear relationships between multiple models. The hierarchy with
Content
as a base model and various models extending it demonstrates good object-oriented design principles. The test appropriately uses UUID for IDs and properly defines constraints such as the unique combination ofuserId
andpostId
in thePostUserLikes
model.
66-68
: Good approach to enhancing the DB with context data.The code correctly creates a tenant and user, then enhances the database with the necessary context information. This approach properly simulates the authentication context needed for testing
@default(auth())
directives.
69-85
: Effective testing of nested creation with automatic tenant association.The test effectively validates the core functionality by creating a post with nested likes in a single operation and asserting that the tenant ID is properly propagated to the nested entities. This is a good regression test for ensuring that foreign key relationships maintain proper tenant context.
packages/runtime/src/enhancements/node/default-auth.ts (1)
129-147
: Good simplification of the foreign key handling logic.The changes simplify the logic for handling foreign key fields during nested data creation. By removing the check for
context.nestingPath.length > 1
, the code now focuses only on the presence of abackLink
and whether the current field is included in the foreign key mapping. This simplification makes the code more maintainable while still correctly handling the core functionality.The updated comment "even if child's fk to parent..." clarifies the behavior and fixes a typo from "event" to "even".
tests/integration/tests/enhancements/proxy/extension-context.test.ts (4)
5-21
: Well-structured schema with simple model definitions.The schema defines two models with clear relationships and permissions. The
Counter
model is appropriately designed with a uniquemodel
field and avalue
field to track counts.
23-62
: Comprehensive extension implementation.The implementation demonstrates extending the Prisma client with both client-level methods (
$one
) and model-level methods (createWithCounter
). The extension properly handles transactions and maintains access to internal Prisma properties.However, I noticed one issue with the comment on line 53:
- if (dbOrTx['$transaction']) { - // not running in a transaction, so we need to create a new transaction + if (dbOrTx['$transaction']) { + // dbOrTx is the client, not a transaction, so we need to create a new transaction return dbOrTx.$transaction(fn); }
64-76
: Effective test coverage for both direct and transactional creation.The test effectively covers both direct creation and creation within transactions, and correctly verifies that the counter is incremented for both cases. The assertion at the end properly validates that the counter value matches the expected count.
37-37
:❓ Verification inconclusive
Consider avoiding reliance on internal Prisma properties.
The test is checking the existence of the internal
_engine
property of Prisma. While this works currently, internal properties may change in future versions of Prisma. Consider using more stable public APIs or documenting that this test may need updates if Prisma internals change.
🏁 Script executed:
#!/bin/bash # Check if the _engine property is still used in the latest version of Prisma rg -A 2 -B 2 "_engine" --type tsLength of output: 1061
Review: Caution on Using Prisma’s Internal
_engine
PropertyThe test in
tests/integration/tests/enhancements/proxy/extension-context.test.ts
(line 37) verifies thatdbOrTx._engine
is defined. Note that our runtime code (e.g., inpackages/runtime/src/enhancements/node/logger.ts
) also accesses this internal property directly. While this approach is effective with the current Prisma version, relying on internal, undocumented properties can lead to breakage when Prisma updates its internals.
- Recommendation: Either switch to stable, public APIs for these tests or add clear documentation/comments that call out the dependency on Prisma internals. This will help future maintainers understand the rationale and quickly identify potential upgrade issues.
packages/runtime/src/enhancements/node/proxy.ts (2)
287-287
: Enhanced proxy creation with parent context.The call to
createHandlerProxy
now includes theproxy
variable as an argument, allowing the handler to reference the parent proxy when needed.
237-237
: Unclear TODO comment about batch transaction support.The TODO comment mentions that batch transactions are not supported yet, but doesn't provide any context about what needs to be done or potential approaches.
Consider elaborating on the TODO comment to provide more context for future developers who might work on implementing batch transaction support. For example:
- What are the challenges in supporting batch transactions?
- Are there any particular design approaches being considered?
- Is there an issue or ticket tracking this enhancement?
-// TODO: batch transaction is not supported yet, how? +// TODO: Batch transaction is not supported yet. +// We need to figure out how to properly proxy the array of Prisma operations +// while maintaining the enhancement capabilities. See issue #XYZ for more details.
No description provided.