-
-
Notifications
You must be signed in to change notification settings - Fork 12
merge dev to main (v3.0.0-alpha.29) #191
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
WalkthroughIntroduces alias-aware query building across runtime dialects and operations, updates computed-field invocation to accept a context with currentModel, revises many-to-many FK ordering, restructures relation tests into granular suites, updates samples and README accordingly, and performs version bumps across packages. No exported API types changed except internal dialect method signatures. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
|
Claude finished @ymc9's task —— View job PR Review: Merge dev to mainI've completed a comprehensive review of this pull request that merges the dev branch to main. This PR contains significant improvements to the runtime engine, particularly around self-relation support and test organization. Todo List:
Key Changes SummaryThis PR introduces self-relation support and significantly improves the codebase organization: ✅ Positive Changes
🔍 Code Quality AssessmentStrengths:
Areas for Improvement:
🚀 Performance & SecurityPerformance:
Security:
🧪 Test CoverageExcellent test additions:
Overall AssessmentThis is a high-quality PR that introduces important self-relation functionality while improving code organization. The self-relation implementation is well-thought-out and handles complex scenarios properly. The test reorganization is exemplary and will significantly improve developer experience. Recommendation: ✅ APPROVE The changes are well-implemented, thoroughly tested, and represent a meaningful improvement to the codebase. The self-relation feature is a valuable addition that maintains consistency with the existing architecture. Minor Suggestions (Non-blocking)
|
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 PR merges the dev branch to main, bumping the version from 3.0.0-alpha.28 to 3.0.0-alpha.29. The primary purpose is to update computed fields to support a context parameter with the current model name, enabling more flexible reference handling in database queries.
Key changes include:
- Updated computed field signatures to accept a context parameter containing the current model name
- Enhanced self-relation handling in many-to-many join table foreign key ordering
- Reorganized relation tests into separate files for better maintainability
Reviewed Changes
Copilot reviewed 34 out of 34 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| Multiple package.json files | Version bump from alpha.28 to alpha.29 |
| packages/sdk/src/ts-schema-generator.ts | Adds context parameter to computed field method signatures |
| samples/blog/ files | Updates computed field usage to use new context parameter |
| packages/runtime/ files | Implements context parameter support and fixes self-relation handling |
| packages/runtime/test/client-api/relation/ | Reorganizes relation tests into separate files by type |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/runtime/src/client/crud/dialects/postgresql.ts (1)
242-249: Fix missing alias in payload.select branch for scalar fieldsIn the
payload.selectbranch of
packages/runtime/src/client/crud/dialects/postgresql.ts
the call tothis.fieldRefis dropping the relation alias:• At line 247, change:
- this.fieldRef(relationModel, field, eb, undefined, false); + this.fieldRef(relationModel, field, eb, relationModelAlias, false);A quick grep confirms this is the only occurrence of
this.fieldRef(..., undefined, false)in this file.packages/runtime/src/client/crud/dialects/sqlite.ts (1)
151-156: Add subquery alias in scalar field selectionsBoth calls to
fieldRefinpackages/runtime/src/client/crud/dialects/sqlite.tscurrently passundefinedfor the alias, which can lead to ambiguous column references and breaks computed fields. Update them to usesubQueryName:• packages/runtime/src/client/crud/dialects/sqlite.ts:154
• packages/runtime/src/client/crud/dialects/sqlite.ts:185Suggested diff:
--- a/packages/runtime/src/client/crud/dialects/sqlite.ts +++ b/packages/runtime/src/client/crud/dialects/sqlite.ts @@ 151,7 # “select all scalar fields” branch - .map(([field]) => [sql.lit(field), this.fieldRef(relationModel, field, eb, undefined, false)]) + .map(([field]) => [sql.lit(field), this.fieldRef(relationModel, field, eb, subQueryName, false)]) @@ 183,7 # “select specific fields” branch - return [ - sql.lit(field), - this.fieldRef(relationModel, field, eb, undefined, false) as ArgsType, - ]; + return [ + sql.lit(field), + this.fieldRef(relationModel, field, eb, subQueryName, false) as ArgsType, + ];This ensures each column reference is correctly scoped to the subquery alias.
🧹 Nitpick comments (13)
packages/cli/package.json (1)
6-6: CLI package version bump LGTMConsider updating release notes/changelog to reflect the new pre-release if you haven’t already.
packages/sdk/src/ts-schema-generator.ts (1)
355-372: Optional: extract a reusable context type to avoid inline duplicationInstead of emitting an inline type literal for every computed field method, consider reusing a named type (e.g.,
ComputedFieldContext) from@zenstackhq/runtime/schemaif available, or emit a local type alias once per file. This reduces duplication and eases future evolution of the context shape.packages/runtime/test/client-api/computed-fields.test.ts (1)
240-245: Nit: Destructure context and align alias to the field name to reduce coupling
- Destructure
currentModeldirectly for brevity.- Alias the count as
postCountto avoid relying on downstream alias remapping. It makes the intent self-evident and resilient to future changes.- postCount: (eb: any, context: { currentModel: string }) => + postCount: (eb: any, { currentModel }: { currentModel: string }) => eb .selectFrom('Post') - .whereRef('Post.authorId', '=', sql.ref(`${context.currentModel}.id`)) - .select(() => eb.fn.countAll().as('count')), + .whereRef('Post.authorId', '=', sql.ref(`${currentModel}.id`)) + .select(() => eb.fn.countAll().as('postCount')),samples/blog/main.ts (1)
9-9: Guard verbose query logging for samplesConsider enabling query logging conditionally so sample users don’t flood output unintentionally.
- log: ['query'], + // Enable with: DEBUG=1 node main.js + log: process.env.DEBUG ? ['query'] : undefined,packages/runtime/test/client-api/relation/one-to-many.test.ts (1)
78-96: Optional: add coverage for filtered includes and orderingTo further validate alias-aware query building, consider adding:
- include with where/orderBy on
posts1/posts2- filtering
Userby conditions on each relation independentlypackages/runtime/test/client-api/relation/one-to-one.test.ts (1)
51-90: Optional: add negative-path validationsConsider extending with:
- attempting to create a second profile on the same named relation to hit the unique constraint
- connect/disconnect flows for 1-1 to ensure aliasing holds for updates too
packages/runtime/test/client-api/relation/many-to-many.test.ts (2)
388-396: Direct join-table inspection: fragile but effectiveChecking
_TagToUser/_${relationName}via$qbRawis a strong assertion. Be aware that:
- The A/B column mapping is provider/implementation-defined; if Prisma or the dialect changes naming, this may break.
- If you want to reduce brittleness, consider asserting via high-level API only, or gate this check per provider/version.
84-113: Optional: isolate sqlite DB names to avoid file lock racesVitest generally runs tests in a file sequentially, but if concurrency is enabled later, both relationName variants could contend on
./test.db(sqlite). Using distinctdbNames per variant avoids potential locking issues.Example:
- dbName: relationName ?
${TEST_DB}-named:${TEST_DB}-unnamedpackages/runtime/src/client/crud/operations/aggregate.ts (1)
66-66: Nit: consider using a named subquery alias constant for consistencyCountOperationHandler uses a subQueryName constant. Here the '$sub' alias is hardcoded in multiple places (e.g., Line 66; later: sql.ref(
$sub.${field}) Lines 84/106). Defining a localconst subQueryName = '$sub'would improve consistency and reduce typos.packages/runtime/src/client/crud/operations/count.ts (1)
15-36: Minor consistency suggestion: reuse subQueryName everywhereYou already use
const subQueryName = '$sub'. If there are any future references to the alias (e.g., in added projections), keep using this constant to avoid divergent literals.packages/runtime/src/client/query-utils.ts (1)
181-182: Computed-field now receives context: tighten typing and clarify namingGood call passing
{ currentModel: modelAlias }. Two suggestions:
- Add a strict function type for computed fields so callers get compile-time help.
- Consider renaming
currentModeltocurrentAliasto avoid conflating model name vs. table alias.Example type to add next to ClientOptions:
export type ComputedFieldFn = ( eb: ExpressionBuilder<any, any>, ctx: { currentModel?: string } ) => ExpressionWrapper<any, any, unknown>;And use it to type
computedFieldslookup instead ofFunction.packages/runtime/test/client-api/relation/self-relation.test.ts (1)
15-31: Stabilize test DB isolation: give each test a unique dbName and consistently force-reset schemaReusing the same dbName across multiple tests and mixing
usePrismaPushtrue/false can cause cross-test contamination (especially for sqlite) and Postgres flakiness when the DB is not pre-created. Use unique db names per test and preferusePrismaPush: trueeverywhere for deterministic resets.Apply these diffs for each test’s client creation:
@@ - { - provider, - dbName: TEST_DB, - usePrismaPush: true, - }, + { + provider, + dbName: `${TEST_DB}-${provider}-o2o`, + usePrismaPush: true, + }, @@ - { - provider, - dbName: TEST_DB, - usePrismaPush: true, - }, + { + provider, + dbName: `${TEST_DB}-${provider}-o2m`, + usePrismaPush: true, + }, @@ - { - provider, - dbName: provider === 'postgresql' ? TEST_DB : undefined, - usePrismaPush: true, - }, + { + provider, + dbName: `${TEST_DB}-${provider}-m2m`, + usePrismaPush: true, + }, @@ - { - provider, - dbName: TEST_DB, - }, + { + provider, + dbName: `${TEST_DB}-${provider}-explicit-m2m`, + usePrismaPush: true, + }, @@ - { - provider, - usePrismaPush: true, - dbName: TEST_DB, - }, + { + provider, + dbName: `${TEST_DB}-${provider}-multi-self`, + usePrismaPush: true, + }, @@ - { - provider, - usePrismaPush: true, - dbName: TEST_DB, - }, + { + provider, + dbName: `${TEST_DB}-${provider}-deep`, + usePrismaPush: true, + },Verification suggested:
- For Postgres, confirm the database creation path is reliable when
usePrismaPush: true. If Prisma doesn't create the DB, consider flipping tousePrismaPush: falsefor Postgres (lettingcreateTestClientprecreate the DB), or precreate DBs in test bootstrap.Also applies to: 122-138, 305-321, 528-552, 604-624, 674-690
packages/runtime/src/client/crud/dialects/base.ts (1)
892-917: Reduce alias collision risk when ordering by to-one relationsLeft-joining the relation table without a unique alias can collide if multiple relations target the same model or when self-relations are involved. Use a per-field alias.
Apply:
- // order by to-one relation - result = result.leftJoin(relationModel, (join) => { - const joinPairs = buildJoinPairs(this.schema, model, modelAlias, field, relationModel); + // order by to-one relation + const relationJoinAlias = `${modelAlias}$orderBy$${field}`; + result = result.leftJoin(`${relationModel} as ${relationJoinAlias}`, (join) => { + const joinPairs = buildJoinPairs(this.schema, model, modelAlias, field, relationJoinAlias); return join.on((eb) => this.and( eb, ...joinPairs.map(([left, right]) => eb(sql.ref(left), '=', sql.ref(right))), ), ); }); - result = this.buildOrderBy(result, fieldDef.type, relationModel, value, false, negated); + result = this.buildOrderBy(result, fieldDef.type, relationJoinAlias, value, false, negated);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (34)
-
README.md(1 hunks) -
package.json(1 hunks) -
packages/cli/package.json(1 hunks) -
packages/common-helpers/package.json(1 hunks) -
packages/create-zenstack/package.json(1 hunks) -
packages/dialects/sql.js/package.json(1 hunks) -
packages/eslint-config/package.json(1 hunks) -
packages/ide/vscode/package.json(1 hunks) -
packages/language/package.json(1 hunks) -
packages/runtime/package.json(1 hunks) -
packages/runtime/src/client/crud/dialects/base.ts(15 hunks) -
packages/runtime/src/client/crud/dialects/postgresql.ts(7 hunks) -
packages/runtime/src/client/crud/dialects/sqlite.ts(6 hunks) -
packages/runtime/src/client/crud/operations/aggregate.ts(1 hunks) -
packages/runtime/src/client/crud/operations/base.ts(5 hunks) -
packages/runtime/src/client/crud/operations/count.ts(1 hunks) -
packages/runtime/src/client/query-utils.ts(2 hunks) -
packages/runtime/test/client-api/computed-fields.test.ts(2 hunks) -
packages/runtime/test/client-api/relation.test.ts(0 hunks) -
packages/runtime/test/client-api/relation/many-to-many.test.ts(1 hunks) -
packages/runtime/test/client-api/relation/one-to-many.test.ts(1 hunks) -
packages/runtime/test/client-api/relation/one-to-one.test.ts(1 hunks) -
packages/runtime/test/client-api/relation/self-relation.test.ts(1 hunks) -
packages/sdk/package.json(1 hunks) -
packages/sdk/src/ts-schema-generator.ts(1 hunks) -
packages/tanstack-query/package.json(1 hunks) -
packages/testtools/package.json(1 hunks) -
packages/typescript-config/package.json(1 hunks) -
packages/vitest-config/package.json(1 hunks) -
packages/zod/package.json(1 hunks) -
samples/blog/main.ts(1 hunks) -
samples/blog/package.json(1 hunks) -
samples/blog/zenstack/schema.ts(1 hunks) -
tests/e2e/package.json(1 hunks)
💤 Files with no reviewable changes (1)
- packages/runtime/test/client-api/relation.test.ts
🧰 Additional context used
📓 Path-based instructions (2)
{packages,samples,tests}/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Packages are located in
packages/,samples/, andtests/
Files:
packages/sdk/package.jsonpackages/runtime/package.jsonpackages/language/package.jsonpackages/testtools/package.jsonpackages/runtime/test/client-api/relation/self-relation.test.tspackages/cli/package.jsonpackages/tanstack-query/package.jsonpackages/vitest-config/package.jsonpackages/typescript-config/package.jsonpackages/create-zenstack/package.jsonpackages/ide/vscode/package.jsonpackages/dialects/sql.js/package.jsontests/e2e/package.jsonpackages/zod/package.jsonpackages/eslint-config/package.jsonpackages/common-helpers/package.jsonpackages/runtime/test/client-api/relation/many-to-many.test.tspackages/runtime/src/client/query-utils.tssamples/blog/zenstack/schema.tspackages/runtime/test/client-api/relation/one-to-many.test.tspackages/runtime/test/client-api/computed-fields.test.tssamples/blog/package.jsonpackages/runtime/test/client-api/relation/one-to-one.test.tspackages/sdk/src/ts-schema-generator.tspackages/runtime/src/client/crud/operations/count.tspackages/runtime/src/client/crud/dialects/postgresql.tspackages/runtime/src/client/crud/dialects/sqlite.tspackages/runtime/src/client/crud/dialects/base.tspackages/runtime/src/client/crud/operations/aggregate.tssamples/blog/main.tspackages/runtime/src/client/crud/operations/base.ts
tests/e2e/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
E2E tests are in
tests/e2e/directory
Files:
tests/e2e/package.json
🧠 Learnings (1)
📚 Learning: 2025-08-04T08:43:33.161Z
Learnt from: CR
PR: zenstackhq/zenstack-v3#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T08:43:33.161Z
Learning: `zenstack generate` compiles ZModel to TypeScript schema (`schema.ts`)
Applied to files:
packages/language/package.json
🧬 Code Graph Analysis (6)
packages/runtime/test/client-api/relation/self-relation.test.ts (3)
packages/runtime/test/utils.ts (1)
createTestClient(78-195)packages/runtime/src/client/crud/dialects/sqlite.ts (1)
provider(26-28)packages/runtime/src/client/crud/dialects/postgresql.ts (1)
provider(26-28)
packages/runtime/test/client-api/relation/many-to-many.test.ts (3)
packages/runtime/test/utils.ts (1)
createTestClient(78-195)packages/runtime/src/client/crud/dialects/sqlite.ts (1)
provider(26-28)packages/runtime/src/client/crud/dialects/postgresql.ts (1)
provider(26-28)
packages/runtime/test/client-api/relation/one-to-many.test.ts (1)
packages/runtime/test/utils.ts (1)
createTestClient(78-195)
packages/runtime/src/client/crud/dialects/postgresql.ts (2)
packages/runtime/src/client/query-utils.ts (1)
buildJoinPairs(197-215)packages/sdk/src/schema/schema.ts (1)
FieldDef(59-74)
packages/runtime/src/client/crud/dialects/base.ts (1)
packages/runtime/src/client/query-utils.ts (7)
requireModel(20-26)makeDefaultOrderBy(217-220)ensureArray(286-292)getManyToManyRelation(222-258)getIdFields(41-44)buildJoinPairs(197-215)getDelegateDescendantModels(322-335)
samples/blog/main.ts (2)
samples/blog/zenstack/schema.ts (1)
schema(9-230)packages/runtime/src/client/functions.ts (1)
currentModel(102-113)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: claude-review
- GitHub Check: build-test (20.x)
🔇 Additional comments (51)
packages/sdk/package.json (1)
3-3: Version bump to 3.0.0-alpha.29 looks goodNo export or dependency changes here; aligns with the repo-wide bump.
packages/runtime/package.json (1)
3-3: Runtime version bump approved — monorepo versions and dependency ranges are consistent
- Verified all @zenstackhq/* packages share version 3.0.0-alpha.29
- Confirmed every internal dependency uses
workspace:*rangesAll checks passed; good to merge.
packages/language/package.json (1)
4-4: Language package version bump approvedMatches the monorepo update; no export surface changes here.
packages/vitest-config/package.json (1)
4-4: Vitest config version bump looks goodNo changes to exports; downstream packages should resolve the new version via workspace.
package.json (1)
3-3: Version bump to 3.0.0-alpha.29 verified across the monorepo
- No remaining
3.0.0-alpha.28occurrences found- All
versionfields in everypackage.jsonare set to3.0.0-alpha.29packageManager: "[email protected]"is correctly pinned in rootpackage.jsonandpackages/ide/vscode/package.jsonpackages/ide/vscode/package.json (1)
4-4: VS Code extension version bump LGTM.Pre-release publish scripts already use --pre-release; no further changes needed here.
packages/zod/package.json (1)
3-3: Zod package version bump is consistent with the monorepo.No API or dependency changes implicated; safe to ship.
packages/testtools/package.json (1)
3-3: Testtools package version bump looks good.No changes to deps or exports; aligns with workspace:* consumers.
packages/typescript-config/package.json (1)
3-3: Typescript-config version bump confirmed.No functional impact; consistent with repo-wide update.
packages/eslint-config/package.json (1)
3-3: Version bump looks goodMetadata-only change to 3.0.0-alpha.29. No concerns.
samples/blog/package.json (1)
3-3: Sample package version bump LGTMAligned to 3.0.0-alpha.29; dependencies remain workspace-pinned. No issues.
packages/common-helpers/package.json (1)
3-3: LGTM: metadata-only version bumpVersion advanced to 3.0.0-alpha.29; no functional changes. Good to go.
packages/create-zenstack/package.json (1)
3-3: LGTM: version bumpBumped to 3.0.0-alpha.29; no other changes. Looks good.
tests/e2e/package.json (2)
3-3: LGTM: version bump to alpha.29Change is scoped to the version field; no other metadata altered. Looks good.
3-3: All clear: no3.0.0-alpha.28remains in the repo
Ran a repo-wide search and allpackage.jsonfiles report version3.0.0-alpha.29. You’re good to merge.packages/tanstack-query/package.json (2)
3-3: LGTM: version bumped to alpha.29No changes to exports, deps, or peerDeps. Safe metadata update.
7-7: Confirm @zenstackhq/tanstack-query’s privacy statusI’ve scanned all 16 package.json files in the repo. The only packages marked
"private": true(and with nopublishConfigoverrides) are:
- packages/eslint-config
- packages/ide/vscode
- packages/typescript-config
- packages/vitest-config
- packages/zod
- tests/e2e
- packages/tanstack-query
If
@zenstackhq/tanstack-queryis meant to remain an internal‐only package, no further changes are required. If you intend to publish it in the future, you’ll need to:
- Remove
"private": true- (Optionally) Add a
publishConfigblock in its package.json- Ensure this change is incorporated into your release workflow
Please confirm your intended visibility for this package.
packages/dialects/sql.js/package.json (2)
3-3: LGTM: version bumped to alpha.29No functional changes in scripts/exports/peerDeps. Looks good.
38-41: PeerDependency alignment confirmed
Verified that in packages/dialects/sql.js/package.json (lines 35–41), both dependencies and peerDependencies specifysql.js: ^1.13.0andkysely: catalog:. The ranges are consistent—no changes needed.packages/sdk/src/ts-schema-generator.ts (1)
355-372: ComputedFields stub signature change is compatible with SchemaDef and client runtimeThe generated methods now take only a
{ currentModel: string }parameter, but:
- SchemaDef’s
computedFields?: Record<string, Function>places no constraints on parameter shape, so stub functions satisfysatisfies SchemaDef.- Runtime client‐side resolvers aren’t using these stubs—they use the user-provided functions typed via
ComputedFieldsOptions, which prepend theExpressionBuilderargument internally.- Existing tests and samples continue to call user-supplied computed field functions as
(eb, context) ⇒ …, so there’s no signature mismatch at runtime.No changes required here.
samples/blog/zenstack/schema.ts (1)
78-81: LGTM: computed field now takes context with currentModelGenerated signature
postCount(_context: { currentModel: string }): OperandExpression<number>matches the SDK change and enforces passing context at call sites.packages/runtime/test/client-api/computed-fields.test.ts (1)
1-1: Importingsqlfor alias-aware refs is correctThis aligns the test with the new computed-field pattern using
sql.ref. Looks good.samples/blog/main.ts (2)
3-3: Correct: bring insqlandSqliteDialectRequired for the dynamic
currentModelref and sample setup. All good.
12-16: Computed field correctly usescurrentModelcontextUsing
sql.ref(\${currentModel}.id`)ensures the join condition stays correct under aliasing. The explicit.as('postCount')` keeps the projection stable.packages/runtime/test/client-api/relation/one-to-many.test.ts (3)
11-13: Good resource cleanupDisconnecting the client after each test prevents handle leaks across providers.
18-30: Schema for unnamed one-to-many is correctThe directional relation and FK are defined properly; nested create + include assertions below exercise the path well.
56-71: Named one-to-many relations are modeled correctlyDual relations with distinct relation names and separate FK fields avoid ambiguity and exercise aliasing changes.
packages/runtime/test/client-api/relation/one-to-one.test.ts (2)
18-30: Unnamed one-to-one schema is soundSingle-owner side with
@uniqueFK is correct; the nested write and include assertions verify the end-to-end flow.
54-69: Named one-to-one relations are correctly disambiguatedSeparate FKs and relation names ensure unambiguous joins. The test validates both branches in one operation.
packages/runtime/test/client-api/relation/many-to-many.test.ts (6)
15-38: Explicit M2M join-table modeling looks correctThe schema uses an explicit join table with a composite unique across
(userId, tagId). The subsequent includes validate both directions.
81-113: Set up for implicit M2M across relationName variants is robustUsing
describe.eachto exercise both unnamed and named implicit M2M is great.usePrismaPush: truewith--force-resetensures clean state per test.
168-176: Filter-by-related scenarios coversomeandnoneThese assertions are valuable to catch alias or subquery mistakes in the new dialect changes.
331-359: Idempotent connect is verifiedAsserting that connecting an already-connected record is a no-op helps guard against duplicate join rows.
446-472:updateManypath validates scoped filteringGood to see the filter excludes id=2 and updates others; this commonly regresses under alias/subquery rewrites.
524-558: Delete vs deleteMany behaviors are well distinguished
deleteremoves the related Tag entity (not just unlink), as asserted for id=2.deleteManyscoped to the current relation removes only connected targets; Tag(3) gone, Tag(1) remains since it’s not connected at that time. Nice coverage.packages/runtime/src/client/crud/operations/aggregate.ts (1)
19-23: buildSelectModel signature usage verified — all calls include the root aliasAll
buildSelectModelcall sites across operations and dialects have been updated to pass three arguments, and the base implementation inpackages/runtime/src/client/crud/dialects/base.tsdeclares the new(eb, model, modelAlias)signature. No lingering two-arg calls or mismatched definitions were found.packages/runtime/src/client/crud/operations/count.ts (1)
18-21: Root alias propagation to buildSelectModel looks goodPassing the third parameter aligns Count with the broader alias-aware changes. No functional concerns here.
packages/runtime/src/client/query-utils.ts (1)
235-245: Many-to-many FK ordering change: deterministic and symmetric — LGTMSwitching to field-name ordering for self-relations removes ambiguity and keeps symmetry across both sides. Join-table naming remains consistent with Prisma conventions. Looks good.
If not already covered, please add/confirm tests for:
- Self-relation M:N with both sides selected to assert A/B stability.
- Cross-model M:N asserting A/B assignment lines up with join-table columns.
Also applies to: 247-251
packages/runtime/src/client/crud/dialects/postgresql.ts (3)
86-101: Subquery aliasing for relations: solid isolationIntroducing
subQueryAliasand passing it throughbuildSelectModel,buildSelectAllFields, andbuildFilterSortTakewill prevent alias collisions (notably self-relations). Well done.
115-129: Join conditions correctly reference the inner aliasUsing
eb.ref(\${subQueryAlias}.${relationIds[0]}`)in M2M and passingsubQueryAliastobuildJoinPairs` for non-M2M ensures correct scoping. This resolves typical self-join alias conflicts.
221-224: Alias-aware scalar selection (select-all path) is correctPassing
relationModelAliasthroughfieldRefretains alias context for both plain and computed fields.packages/runtime/src/client/crud/dialects/sqlite.ts (2)
79-94: Subquery aliasing propagation is correctDefining
subQueryAliasand threading it through select/filter/sort/take prevents alias clashes in self-relations. Good alignment with the Postgres approach.
164-176: Count and nested relation JSON calls updated to direct relation model — LGTMDropping unnecessary type assertions and consistently using the relation model improves readability without changing semantics.
packages/runtime/test/client-api/relation/self-relation.test.ts (1)
6-14: Ensure Postgres databases exist when usingusePrismaPush: true
createTestClientonly drop/creates Postgres DBs whenusePrismaPushis false. WithusePrismaPush: true, Prisma may not create the database if missing. Make sure test setup pre-creates the DB or useusePrismaPush: falsefor Postgres runs to rely on the helper’s DB creation.If your CI does not precreate DBs, flip Postgres tests to
usePrismaPush: falseor add a pre-step to create${TEST_DB}-postgresql-*databases before tests.packages/runtime/src/client/crud/operations/base.ts (3)
147-151: Alias propagation in read-path is solidPassing
modelasmodelAliasintobuildSelectModel,buildFilterSortTake, andbuildSelectAllFieldsaligns with the dialect’s new alias-aware API and prevents collisions in self-joins.Also applies to: 159-160
487-492: Correct m2m FK ordering for self-relationsSorting by model name and then field name for identical models matches Prisma’s convention for A/B ordering and fixes ambiguous self-m2m mapping.
1289-1294: ✅ Verified alias-aware subselects for updateMany/delete fallbacksAll call sites of
buildSelectModel,buildFilterSortTake, andbuildSelectAllFieldshave been updated to the new signature and aliasing conventions:• packages/runtime/src/client/crud/operations/base.ts (lines 1289–1294, 1993–1997)
• packages/runtime/src/client/crud/operations/aggregate.ts
• packages/runtime/src/client/crud/operations/count.ts
• packages/runtime/src/client/crud/dialects/base.ts
• packages/runtime/src/client/crud/dialects/sqlite.ts
• packages/runtime/src/client/crud/dialects/postgresql.tsNo outdated usages remain.
packages/runtime/src/client/crud/dialects/base.ts (4)
50-57: Good: root table aliasing and delegate-base joins
buildSelectModelnow cleanly aliases the root table and joins delegate bases usingthisModelAlias, preventing ambiguous refs in multi-join scenarios.
170-203: Cursor subquery isolation is correctIntroducing
subQueryAliasand building the cursor filter against the inner alias prevents alias collisions and produces stable pagination semantics.
925-942: Alias-aware field selection across base and delegate descendants looks goodForwarding
modelAliasthroughbuildSelectAllFieldsandbuildDelegateJoinmaintains consistency and prevents ambiguous column refs.Also applies to: 947-961
984-999: Delegate join signature change is clear and consistentJoining on id fields using
thisModelAliasandotherModelAliasmatches the rest of the aliasing strategy.
Summary by CodeRabbit