ORM commands now state all four output channels instead of letting the engine fill them in - #30004
Merged
Merged
Conversation
`Presentations` in @prisma/cli-engine 0.0.9 makes `stdout`, `json` and `next` optional, and these commands took that up: 21 files declared `human` and `json` and left the other two to the engine's defaults. The engine is making all four required, so a command states each surface it publishes rather than inheriting one by omission. Until that version is pinned here the consumer has to call the missing ones defensively, which is a shim standing in for a declaration we can simply write. Behaviour is unchanged: every added `stdout` and `next` returns the empty array the engine already substituted. The point is that the omission becomes visible, so a command that should publish machine-readable lines can be given them one at a time. Verified against the stricter type by making the four required in the installed engine's declarations and typechecking: no source file is missing one. A test fixture in define-command.test.ts was the only other site. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
|
|
prisma-next
@prisma/orm-extension-arktype-json
@prisma/orm-extension-middleware-cache
@prisma/orm-extension-paradedb
@prisma/orm-extension-pgvector
@prisma/orm-extension-postgis
@prisma/orm-extension-supabase
@prisma/orm-family-mongo
@prisma/orm-family-sql
@prisma/orm-framework
@prisma/orm-mongo
@prisma/orm-postgres
@prisma/orm-sqlite
@prisma/orm-target-mongo
@prisma/orm-target-postgres
@prisma/orm-target-sqlite
@prisma/orm-toolchain
commit: |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (22)
📝 WalkthroughWalkthroughORM CLI presentation objects now include empty ChangesORM presentation normalization
Estimated code review effort: 2 (Simple) | ~10 minutes Mergeability Score: ⚪ Minimal · up to This localized change makes command output declarations explicit without changing intended behavior, and no actionable merge-blocking risk remains after normal checks and review. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
Contributor
size-limit report 📦
|
wmadden-electric
changed the title
wmadden
approved these changes
Aug 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Here is the entire change, repeated across 21 files:
// src/orm/migration/list.ts function listPresentations(inputs: { … }): Presentations { return { human: (): readonly Block[] => [ { kind: 'fields', rail: true, rows: [ … ] }, { kind: 'drawing', lines: toneDrawing(inputs.tree) }, ], + stdout: () => [], + next: () => [], json: () => inputs.list, }; }What those four things are
Every CLI command hands the engine four ways of reporting what it did, and the engine uses the ones the run needs:
humanstdoutjsonresultinside the--jsonenvelopenextToday
humanis required and the other three are optional. When a command leaves one out, the engine substitutes a default — an empty list forstdoutandnext.The decision
The engine is dropping those defaults and requiring all four. A command will state what it publishes on each channel rather than inheriting it by omission.
This PR does that stating for the ORM commands. Nothing else changes.
Why it matters, concretely
Once the engine stops substituting, it calls whatever the command declared. Our commands declare two of the four, so it calls functions that are not there:
That is
stdoutin human mode — the default for anyone at a terminal — andnextin both modes. It affects the 21 command files in this diff.Worth noting how it was found, because it says something about our coverage: this package's own suite passes either way. It never sees the engine version it will be mounted against. The failure only appears when you run the assembled
prismabinary, which neither repo tests today.Behaviour is identical
Every added
stdoutandnextreturns the same empty array the engine was already substituting. This is a no-op at runtime, on purpose.What it buys is visibility.
stdoutis the channel a script reads, and today every ORM command returns nothing on it — not as a decision, but because nobody wrote one. After this, that gap is a visible() => []in each file, and a command that ought to emit machine-readable lines can be given them one at a time, deliberately and reviewably.Timing: this does not wait on anything
The three fields are optional in the engine we pin (
0.0.9) and in the newest published engine (8.0.0-rc.1, on thenexttag) alike. The version that requires them is not published yet — it is prisma-cli#171, still open. So this can merge and ship now, and the engine can then require the fields without breaking us.How I know nothing was missed
I made the four fields required in the installed engine's type declarations and typechecked the package against that — the exact condition the new engine imposes. Zero errors across
src/afterwards, and it caught one site a grep had missed: a fixture intest/orm/define-command.test.ts. The declarations were then restored, so nothing in this diff touchesnode_modules.pnpm build,typecheckandlintclean; 155 test files, 1943 tests passing.Alternatives considered
Handle it in prisma-cli instead, by calling the missing channels defensively. That shim exists today and works. Rejected as the permanent answer: it stands in for a declaration we can simply write in our own code, and it keeps the engine guessing on behalf of every ORM command we add from here.
Wait for the strict engine to publish and do this together with the version bump. Rejected: the fields are optional in every published engine, so this ships now. Waiting only keeps the consumer shimmed for longer and couples two releases that need not be coupled.
Bump the engine pin from
0.0.9to8.0.0-rc.1in the same PR. Worth doing — an install of@prisma/clicurrently carries two copies of the engine because this package pins0.0.9while prisma-cli builds8.0.0-rc.1— but it is a large version jump with its own surface changes, and it is not needed to make the presentations correct. Kept separate so this diff stays a reviewable no-op.Give the commands real
stdoutoutput now rather than() => []. Rejected for this PR. Whatmigration listshould print to a pipe is a genuine per-command design question and a behaviour change deserving its own review.() => []is exactly what ships today, which keeps this diff a no-op reviewable in one pass.