Fix stale cache-hit lifecycle claims in middleware doc comments#915
Open
sorenbs wants to merge 1 commit into
Open
Fix stale cache-hit lifecycle claims in middleware doc comments#915sorenbs wants to merge 1 commit into
sorenbs wants to merge 1 commit into
Conversation
The prisma-next-demo db.ts comment claimed that registering the cache first makes a hit short-circuit downstream middleware (lints, budgets), and find-user-by-id-cached.ts referenced a telemetry middleware that does not exist. Per run-with-middleware.ts, beforeExecute runs for every middleware before any intercept is consulted; a hit skips only the driver call and onRow hooks, and afterExecute still fires for all middleware with source: middleware. Reword both example comments accordingly, point the observability note at the real slowQueryWarning middleware, and fix the same beforeExecute-is-skipped claim in the middleware-cache package doc, which likely seeded the example wording. Found while validating the Prisma Next docs (prisma/web#8008). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Søren Bramer Schmidt <sorenbs@gmail.com>
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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/extension-supabase
@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/config-loader
@prisma-next/emitter
@prisma-next/language-server
@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: |
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
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.
Linked issue
Refs prisma/web#8008 (Prisma Next docs validation, which surfaced these stale comments). Stacked on #912 (
examples/extensions-middleware-docs) — the corrected comments reference itsslowQueryWarningexample, so that PR must merge first. Sibling fix from the same validation pass: #913.At a glance
Before, this comment claimed a cache hit "short-circuits before any downstream middleware (
lints,budgets) fires" — behavior the framework does not have.Decision
Comment-only PR (no runtime change): correct three doc comments that describe middleware-lifecycle behavior the framework does not have.
onRow, nothing else).afterExecutefires for every middleware on a hit, taggedsource: 'middleware') and points atslowQueryWarning, the actual observer registered in db.ts.beforeExecute,runDriver, andonRow") and is the likely origin of the stale example wording. Fixed thebeforeExecuteclaim.Reviewer notes
packages/3-extensions/middleware-cache/src/cache-middleware.ts) is outside the examples scope the validation flagged, but it carries the identical false claim and likely seeded it. One-sentence doc fix; happy to split it out if you prefer this PR examples-only.mainautomatically (or I can rebase).examples/and the cache package: they all use the term in the accurate sense (the driver is skipped) and stand unchanged.TML-NNNN:convention; left unchecked in the checklist below.lint-depshook could not run locally (dependency-cruiser requires Node ≥ 24; the local shell had 23.7), so the commit bypassed it. The diff is comment-only with imports untouched; CI runs the authoritative check.How the lifecycle actually works
The corrected wording follows the execution order that run-with-middleware.ts documents as canonical:
beforeExecuteruns first, for every middleware. Family runtimes callrunBeforeExecuteChainbetween AST → plan lowering and parameter encoding (sql-runtime.ts, before-execute-chain.ts), solintsandbudgetsvet every execution before any cache lookup happens.interceptis consulted afterwards, in registration order; the first non-undefinedresult wins. That precedence is the actual reason to register the cache first.onRowhooks — nothing else.afterExecutestill fires for every middleware withsource: 'middleware', which is how observers such as slow-query-warning.ts see cached reads.What changed & evidence
driver.execute).slowQueryWarninginstead of a nonexistent telemetry middleware. TheafterExecute-fires-on-hit behavior is pinned by cache-middleware.test.ts.beforeExecuteis skipped on a hit.Testing performed
pnpm --filter prisma-next-demo --filter @prisma-next/middleware-cache typecheck— passes. Comment-only change with no behavioural delta, so no test updates.Skill update
n/a — doc comments only; no user-facing surface (CLI, public API, config, error codes) changed.
Alternatives considered
lints/budgets): rejected — runningbeforeExecutebefore parameter encoding is deliberate, so middleware that mutatesParamRefvalues (e.g. bulk-encrypt) stays visible to encode;run-with-middleware.tsdocuments this lifecycle as canonical.afterExecutestill fires on hits, taggedsource: 'middleware') is the useful teaching point, and Add slow-query warning custom middleware example to prisma-next-demo #912'sslowQueryWarningfinally gives it a real referent.examples/and leave the package doc stale: rejected — it is the same false claim and the likely origin; fixing it here keeps one logical concern in one review.Checklist
git commit -s) per the DCO.TML-NNNN:form — no Linear ticket exists for this fix; plain title per the precedent of Add slow-query warning custom middleware example to prisma-next-demo #912/docs: update Supabase runtime README #913 (see reviewer notes).