TML-2596: add OrderByItem.reverse() for backward cursor pagination#671
Conversation
|
Warning Review limit reached
More reviews will be available in 24 minutes and 36 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughA single instructions file is updated to include upgrade metadata marking the version transition from 0.12 to 0.13, with ChangesUpgrade Metadata for 0.12 to 0.13 Transition
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
size-limit report 📦
|
@prisma-next/extension-author-tools
@prisma-next/mongo-runtime
@prisma-next/family-mongo
@prisma-next/sql-runtime
@prisma-next/family-sql
@prisma-next/extension-arktype-json
@prisma-next/middleware-cache
@prisma-next/mongo
@prisma-next/extension-paradedb
@prisma-next/extension-pgvector
@prisma-next/extension-postgis
@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/ts-render
@prisma-next/contract-authoring
@prisma-next/ids
@prisma-next/psl-parser
@prisma-next/psl-printer
@prisma-next/cli
@prisma-next/cli-telemetry
@prisma-next/emitter
@prisma-next/migration-tools
prisma-next
@prisma-next/vite-plugin-contract-emit
@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-query-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: |
93663e6 to
b02c6f7
Compare
aqrln
left a comment
There was a problem hiding this comment.
Lint failure on CI:
check-upgrade-coverage: 1 violation(s) (0.12 → 0.12)
[coverage] diff in packages/3-extensions/ requires an upgrade-instructions directory at
skills/extension-author/prisma-next-extension-upgrade/upgrades/0.12-to-0.13/instructions.md
sample paths from the diff:
packages/3-extensions/sql-orm-client/README.md
See the in-repo `record-upgrade-instructions` skill for the authoring workflow:
.agents/skills/record-upgrade-instructions/SKILL.md
a293064 to
77cf1ee
Compare
…ation Add an OrderByItem.reverse() instance method that returns a new frozen item with the sort direction flipped and expr unchanged, and re-export OrderByItem from the public ORM client surface so integrations that own pagination (Relay backward cursors, REST list endpoints) can invert a user sort order without reaching into @prisma-next/sql-relational-core/ast. TML-2596 Signed-off-by: Alexey Orlenko's AI Agent <robot@aqrln.net>
…-export Direction Address code review: the README backward-pagination snippet now reverses inside the orderBy selector (the form the public chain API actually accepts) instead of mapping a bare OrderByItem[] that no public method consumes. Also re-export Direction so consumers can name the type that .dir / .reverse() flips between, without reaching into sql-relational-core. TML-2596 Signed-off-by: Alexey Orlenko's AI Agent <robot@aqrln.net>
…as the API Re-exporting OrderByItem (and Direction) from the ORM client singled out two AST types out of the whole alphabet that otherwise lives only in @prisma-next/sql-relational-core/ast. .reverse() being an instance method is enough: integrations call it on the item the orderBy selector already returns, so no value-level import of OrderByItem is needed. Move the public-shape type test to relational-core (OrderByItem.reverse() returns OrderByItem; readable dir/expr) next to the runtime round-trip test; drop the now-redundant sql-orm-client tests; reword the README pagination note to use .reverse() inline. TML-2596 Signed-off-by: Alexey Orlenko's AI Agent <robot@aqrln.net>
The type assertions just restated the class definition (reverse(): OrderByItem, dir: Direction, expr: AnyExpression) without exercising any type-level logic. The runtime round-trip + frozen-identity coverage in order.test.ts is the meaningful test. Per PR review. TML-2596 Signed-off-by: Alexey Orlenko's AI Agent <robot@aqrln.net>
.reverse() is for extension/integration authors, not ORM-client users, so the backward-cursor example and its prose do not belong in this README. Keep the forward orderBy example only. Per PR review. TML-2596 Signed-off-by: Alexey Orlenko's AI Agent <robot@aqrln.net>
This PR's only packages/3-extensions/ diff is a doc-only trim of the sql-orm-client README pagination section — no consumer-side action. Record the in-flight transition with changes: [] to satisfy the upgrade-coverage gate, per the record-upgrade-instructions skill. TML-2596 Signed-off-by: Alexey Orlenko's AI Agent <robot@aqrln.net>
77cf1ee to
a6ed2f2
Compare
Linked issue
Refs TML-2596
At a glance
Before this PR,
OrderByItemexposed no inversion path. An integration that owns pagination (Relay backward cursors, REST list endpoints) could onlynew OrderByItem(item.expr, item.dir === 'asc' ? 'desc' : 'asc')against the internal@prisma-next/sql-relational-core/astpackage — or abandon the chain API entirely, which is what the Pothos and Drizzle plugins ended up doing.Decision
Add an instance method
OrderByItem.prototype.reverse()that returns a new frozen item with the direction flipped (asc⇄desc) andexprunchanged.Directionis'asc' | 'desc'only (noNULLS FIRST/LASTyet), so a single-axis flip is unambiguous.That's the whole API. Because
.reverse()is an instance method, an integration flips a sort by calling it on the item theorderByselector already returns (selector(post).reverse()) — it never has to name or importOrderByItemat the value level. So this PR deliberately does not re-exportOrderByItemfrom@prisma-next/sql-orm-client(see Alternatives).How it fits together
reverse()to theOrderByItemclass inrelational-core/src/ast/types.ts. It reuses the frozen-class constructor, so the returned item carries the same immutability guarantees as.asc()/.desc().orderByselector (the form the chain API accepts) — no AST import in the example.Behavior changes & evidence
OrderByItem.reverse()returns a new frozen item with the opposite direction and the sameexprreference —types.ts. Covered by the round-trip + frozen-identity runtime cases inorder.test.ts, with the public type shape (reverse(): OrderByItem, readabledir/expr) locked byorder.types.test-d.ts.Reviewer notes
OrderByItem(per the ticket's original proposal). On review we dropped that: it would single out two AST types (OrderByItem,Direction) from the whole alphabet that otherwise lives only insql-relational-core/ast, and.reverse()as an instance method removes the need for a value-level import anyway. The type test therefore lives inrelational-corenext to the runtime test, not insql-orm-client.Compatibility / migration / risk
Purely additive — one new method. No existing signatures change; no migration needed.
Verification
pnpm --filter @prisma-next/sql-relational-core test— 289 passedpnpm --filter @prisma-next/sql-relational-core typecheck— clean (includes the new.test-d.ts)pnpm --filter @prisma-next/sql-orm-client test— 505 passed, 3 skipped; type tests: no errorsAlternatives considered
OrderByItem/Directionfrom@prisma-next/sql-orm-client(the ticket's original step 2). Rejected: it exposes a two-type slice of the AST alphabet from the ORM surface while every other AST type stays insql-relational-core/ast— an inconsistent boundary..reverse()makes the re-export unnecessary for the runtime path; consumers that want the type for annotations can still name it fromsql-relational-core/ast.OrderByItem.invert(item)helper instead of an instance method. The instance method composes naturally with the chain API and matches the existing.asc()/.desc()ergonomics, so it's the more discoverable surface.orderBy: [...](what Hayes built in his plugin). A separate design question tracked as a follow-up; this PR only fills the immediate gap in the chain API.Checklist
git commit -s) per the DCO.TML-NNNN: <sentence-case title>form.Skill update
n/a — this adds a public method covered by README and tests; no CLI/config/error-code surface changed that the agent skills track.
Summary by CodeRabbit