feat(sql-orm-client): expand and simplify output types#283
feat(sql-orm-client): expand and simplify output types#283
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughA new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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)
Comment |
@prisma-next/mongo-runtime
@prisma-next/family-mongo
@prisma-next/sql-runtime
@prisma-next/family-sql
@prisma-next/middleware-telemetry
@prisma-next/extension-paradedb
@prisma-next/extension-pgvector
@prisma-next/postgres
@prisma-next/sql-orm-client
@prisma-next/sqlite
@prisma-next/target-mongo
@prisma-next/adapter-mongo
@prisma-next/driver-mongo
@prisma-next/contract
@prisma-next/utils
@prisma-next/config
@prisma-next/errors
@prisma-next/framework-components
@prisma-next/operations
@prisma-next/contract-authoring
@prisma-next/ids
@prisma-next/psl-parser
@prisma-next/psl-printer
@prisma-next/cli
@prisma-next/emitter
@prisma-next/migration-tools
@prisma-next/vite-plugin-contract-emit
@prisma-next/runtime-executor
@prisma-next/mongo-codec
@prisma-next/mongo-contract
@prisma-next/mongo-value
@prisma-next/mongo-contract-psl
@prisma-next/mongo-contract-ts
@prisma-next/mongo-emitter
@prisma-next/mongo-schema-ir
@prisma-next/mongo-query-ast
@prisma-next/mongo-orm
@prisma-next/mongo-pipeline-builder
@prisma-next/mongo-lowering
@prisma-next/mongo-wire
@prisma-next/sql-contract
@prisma-next/sql-errors
@prisma-next/sql-operations
@prisma-next/sql-schema-ir
@prisma-next/sql-contract-psl
@prisma-next/sql-contract-ts
@prisma-next/sql-contract-emitter
@prisma-next/sql-lane-query-builder
@prisma-next/sql-relational-core
@prisma-next/sql-builder
@prisma-next/target-postgres
@prisma-next/target-sqlite
@prisma-next/adapter-postgres
@prisma-next/adapter-sqlite
@prisma-next/driver-postgres
@prisma-next/driver-sqlite
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/3-extensions/sql-orm-client/test/simplify-deep.test-d.ts (1)
74-75: Use double cast for test mocks.Per coding guidelines, test mocks should use double casts (
as unknown as X) to make the unsafe boundary explicit.♻️ Suggested fix
const runtime = createMockRuntime(); - const context = {} as ExecutionContext<TestContract>; + const context = {} as unknown as ExecutionContext<TestContract>;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/3-extensions/sql-orm-client/test/simplify-deep.test-d.ts` around lines 74 - 75, The test mock `context` is using a single cast which hides the unsafe conversion; change the declaration where `context` is assigned (`const context = {} as ExecutionContext<TestContract>`) to use a double cast (`as unknown as ExecutionContext<TestContract>`) so the unsafe boundary is explicit; leave `createMockRuntime()` as-is and update only the `context` variable's casting.packages/3-extensions/sql-orm-client/src/types.ts (1)
28-35: Readonly arrays lose theirreadonlymodifier.The array branch returns
SimplifyDeep<Element>[](mutable), which stripsreadonlyfrom input types likereadonly T[]. If any row fields are typed as readonly arrays, the simplified type will become mutable.If preserving readonly is desired:
♻️ Suggested fix to preserve readonly
export type SimplifyDeep<T> = T extends readonly (infer Element)[] - ? SimplifyDeep<Element>[] + ? readonly SimplifyDeep<Element>[] : T extends string | number | boolean | bigint | symbol | Date | Uint8Array ? T : T extends object ? { [K in keyof T]: SimplifyDeep<T[K]> } : T;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/3-extensions/sql-orm-client/src/types.ts` around lines 28 - 35, The SimplifyDeep utility is stripping readonly from array types because the array branch returns SimplifyDeep<Element>[] (mutable); update the array branch in SimplifyDeep so it preserves readonly (e.g., when T extends readonly (infer Element)[] return a readonly array of SimplifyDeep<Element>), ensuring readonly T[] inputs remain readonly in the output; modify the SimplifyDeep definition (the branch matching readonly (infer Element)[]) to produce a readonly SimplifyDeep<Element>[] rather than a mutable array.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/3-extensions/sql-orm-client/src/types.ts`:
- Around line 28-35: The SimplifyDeep utility is stripping readonly from array
types because the array branch returns SimplifyDeep<Element>[] (mutable); update
the array branch in SimplifyDeep so it preserves readonly (e.g., when T extends
readonly (infer Element)[] return a readonly array of SimplifyDeep<Element>),
ensuring readonly T[] inputs remain readonly in the output; modify the
SimplifyDeep definition (the branch matching readonly (infer Element)[]) to
produce a readonly SimplifyDeep<Element>[] rather than a mutable array.
In `@packages/3-extensions/sql-orm-client/test/simplify-deep.test-d.ts`:
- Around line 74-75: The test mock `context` is using a single cast which hides
the unsafe conversion; change the declaration where `context` is assigned
(`const context = {} as ExecutionContext<TestContract>`) to use a double cast
(`as unknown as ExecutionContext<TestContract>`) so the unsafe boundary is
explicit; leave `createMockRuntime()` as-is and update only the `context`
variable's casting.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 5f49d154-0964-462d-a77d-f8515e353d75
📒 Files selected for processing (4)
packages/3-extensions/sql-orm-client/src/collection.tspackages/3-extensions/sql-orm-client/src/grouped-collection.tspackages/3-extensions/sql-orm-client/src/types.tspackages/3-extensions/sql-orm-client/test/simplify-deep.test-d.ts
aqrln
force-pushed
the
tml-2139-prettify-query-result-types-with-recursive-mapped-type
branch
from
131787e to
c25c1dd
Compare
wmadden
left a comment
wmadden
left a comment
There was a problem hiding this comment.
Preemptively approving — the approach is sound and the implementation is clean. Please address or respond to the inline comments before merging; no re-review needed.
Blocking (2): F01 (double-wrapping at each chaining step — apply at terminal methods instead), F02 (add test for deeply chained operations).
Non-blocking (3): F03 (readonly array erasure), F04 (raise to @prisma-next/utils), F05 (polymorphic model forward risk).
| export class Collection< | ||
| TContract extends SqlContract<SqlStorage>, | ||
| ModelName extends string, | ||
| Row = DefaultModelRow<TContract, ModelName>, | ||
| Row = SimplifyDeep<DefaultModelRow<TContract, ModelName>>, | ||
| State extends CollectionTypeState = DefaultCollectionTypeState, |
There was a problem hiding this comment.
F01 — Blocking: SimplifyDeep applied at every chaining step instead of terminal methods
The default Row is already SimplifyDeep<DefaultModelRow<...>>. When include() returns SimplifyDeep<Row & { posts: ... }>, this becomes SimplifyDeep<SimplifyDeep<...> & { ... }>. Each chaining step adds another wrapper. For users.select('name').include('posts'), that's three nested SimplifyDeep evaluations — O(N×K) work for N chained ops with K total keys.
Suggestion: Apply SimplifyDeep only at the terminal methods (first(), all(), toArray()) instead of at each builder step. Builder methods accumulate raw intersections internally; only the final result type gets simplified. This reduces overhead to exactly one pass regardless of chain length and is architecturally cleaner — simplification is a presentation concern, not a builder concern.
There was a problem hiding this comment.
Each SimplifyDeep step receives AlreadyFlatObject & { newField: ... } as input — not a deeply nested tree. TypeScript caches the resolution of the inner SimplifyDeep, so step 2 isn't re-traversing step 1's keys. Look at what actually happens:
SimplifyDeep<
Row & {Row is already a resolved flat mapped type from the previous step. The outer SimplifyDeep just flattens one intersection of FlatObj & { posts: ... } — a single pass over the combined keys, not a re-walk of the entire history.
Also, if you defer to terminal methods, then hovering over any intermediate builder step shows raw intersections like { id: number; name: string; ... } & { posts: PostRow[] } & { invitedBy: UserRow | null }.
Closes: https://linear.app/prisma-company/issue/TML-2139/prettify-query-result-types-with-recursive-mapped-type Query result types in `sql-orm-client` are currently unexpanded intersections full of internal utility types, making them hard to read in IDE tooltips and error messages. This change applies a recursive mapped type at the top-level return positions so that IDE tooltips and hover types display fully-evaluated object literals.
aqrln
force-pushed
the
tml-2139-prettify-query-result-types-with-recursive-mapped-type
branch
from
c25c1dd to
dc9edce
Compare
aqrln
force-pushed
the
tml-2139-prettify-query-result-types-with-recursive-mapped-type
branch
from
0005e2d to
55cb9a9
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/1-framework/0-foundation/utils/test/simplify-deep.test-d.ts (1)
17-20: Use strict type-identity checks for branded types instead ofexpectTypeOf.Line 19 uses
toEqualTypeOf()which performs structural comparison and erases brand information, risking false positives where the brand erosion goes undetected. The test explicitly asserts "branded primitives pass through," so strict type-identity verification is essential.💡 Proposed refactor
test('branded primitives pass through', () => { type Branded = string & { readonly __brand: true }; - expectTypeOf<SimplifyDeep<Branded>>().toEqualTypeOf<Branded>(); + type Equal<A, B> = (<T>() => T extends A ? 1 : 2) extends + (<T>() => T extends B ? 1 : 2) + ? true + : false; + type Expect<T extends true> = T; + type _brandedPreserved = Expect<Equal<SimplifyDeep<Branded>, Branded>>; });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/1-framework/0-foundation/utils/test/simplify-deep.test-d.ts` around lines 17 - 20, The test currently uses expectTypeOf<SimplifyDeep<Branded>>().toEqualTypeOf<Branded>() which does a structural compare and can lose the brand; replace this with a strict type-identity assertion: create or reuse a compile-time IsIdentical<A,B> utility and assert IsIdentical<SimplifyDeep<Branded>, Branded> is true (e.g. via a type-level Assert or conditional type that fails compilation if not identical) so the branded primitive identity is enforced for the test named "branded primitives pass through" and references SimplifyDeep and Branded.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/1-framework/0-foundation/utils/test/simplify-deep.test-d.ts`:
- Line 14: The test uses the type-form assertion
expectTypeOf<SimplifyDeep<never>>().toEqualTypeOf<never>() which fails with the
current Vitest; replace it with the documented value-form workaround used
elsewhere: create a value cast to SimplifyDeep<never> (e.g. use a plain value
cast like undefined as SimplifyDeep<never>) and call expectTypeOf on that value,
then assert equality to never via toEqualTypeOf; apply the same change pattern
where you see the same failing assertion (look for
expectTypeOf<...>().toEqualTypeOf<never> and SimplifyDeep usages).
---
Nitpick comments:
In `@packages/1-framework/0-foundation/utils/test/simplify-deep.test-d.ts`:
- Around line 17-20: The test currently uses
expectTypeOf<SimplifyDeep<Branded>>().toEqualTypeOf<Branded>() which does a
structural compare and can lose the brand; replace this with a strict
type-identity assertion: create or reuse a compile-time IsIdentical<A,B> utility
and assert IsIdentical<SimplifyDeep<Branded>, Branded> is true (e.g. via a
type-level Assert or conditional type that fails compilation if not identical)
so the branded primitive identity is enforced for the test named "branded
primitives pass through" and references SimplifyDeep and Branded.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 8cb11c27-2103-46a9-8ba2-a1bc417083a9
📒 Files selected for processing (3)
packages/1-framework/0-foundation/utils/src/simplify-deep.tspackages/1-framework/0-foundation/utils/test/simplify-deep.test-d.tspackages/1-framework/0-foundation/utils/vitest.config.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/1-framework/0-foundation/utils/src/simplify-deep.ts
| expectTypeOf<SimplifyDeep<null>>().toEqualTypeOf<null>(); | ||
| expectTypeOf<SimplifyDeep<undefined>>().toEqualTypeOf<undefined>(); | ||
| expectTypeOf<SimplifyDeep<unknown>>().toEqualTypeOf<unknown>(); | ||
| expectTypeOf<SimplifyDeep<never>>().toEqualTypeOf<never>(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify existing never-assertion patterns used across declaration tests.
rg -nP --type=ts 'toEqualTypeOf<never>\(\)' -g '**/*.test-d.ts'
rg -nP --type=ts 'toEqualTypeOf\(undefined as never\)' -g '**/*.test-d.ts'Repository: prisma/prisma-next
Length of output: 604
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check if type test files are included in test configuration
if [ -f "vitest.config.ts" ] || [ -f "vitest.config.mts" ]; then
echo "=== Vitest Configuration ==="
head -50 vitest.config.* 2>/dev/null | grep -A5 -B5 "test-d" || echo "No test-d pattern found in config start"
fi
# Count all instances of the problematic pattern
echo "=== Pattern: toEqualTypeOf<never>() instances ==="
rg -c 'toEqualTypeOf<never>\(\)' -g '**/*.test-d.ts' || echo "0"
# List files that need fixing
echo "=== Files using problematic pattern ==="
rg -l 'toEqualTypeOf<never>\(\)' -g '**/*.test-d.ts'
# Check for any type test errors in the build
if [ -f "pnpm-lock.yaml" ]; then
echo "=== Package manager: pnpm detected ==="
fiRepository: prisma/prisma-next
Length of output: 474
Use the documented workaround pattern for never type assertions.
Line 14 uses toEqualTypeOf<never>(), which does not compile with the current Vitest version. Replace with the value-form workaround as already used in the integration test suite.
Proposed fix
- expectTypeOf<SimplifyDeep<never>>().toEqualTypeOf<never>();
+ expectTypeOf<SimplifyDeep<never>>().toEqualTypeOf(undefined as never);Note: The same issue exists in packages/2-mongo-family/1-foundation/mongo-codec/test/codecs.test-d.ts:51 and should be fixed with the same pattern.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| expectTypeOf<SimplifyDeep<never>>().toEqualTypeOf<never>(); | |
| expectTypeOf<SimplifyDeep<never>>().toEqualTypeOf(undefined as never); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/1-framework/0-foundation/utils/test/simplify-deep.test-d.ts` at line
14, The test uses the type-form assertion
expectTypeOf<SimplifyDeep<never>>().toEqualTypeOf<never>() which fails with the
current Vitest; replace it with the documented value-form workaround used
elsewhere: create a value cast to SimplifyDeep<never> (e.g. use a plain value
cast like undefined as SimplifyDeep<never>) and call expectTypeOf on that value,
then assert equality to never via toEqualTypeOf; apply the same change pattern
where you see the same failing assertion (look for
expectTypeOf<...>().toEqualTypeOf<never> and SimplifyDeep usages).
2 participants
Closes: https://linear.app/prisma-company/issue/TML-2139/prettify-query-result-types-with-recursive-mapped-type
Query result types in
sql-orm-clientare currently unexpanded intersections full of internal utility types, making them hard to read in IDE tooltips and error messages.This change applies a recursive mapped type at the top-level return positions so that IDE tooltips and hover types display fully-evaluated object literals.
Before:
After:
Summary by CodeRabbit
Refactor
Tests
Chores