-
Notifications
You must be signed in to change notification settings - Fork 868
docs: rewrite Deno Deploy guides with Prisma 7 + Deno 2 patterns #7414
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
WalkthroughThese changes update Prisma documentation for Deno integration and deployment, introducing runtime-specific generator configuration, PrismaPg adapter usage, deno.json setup with tasks, and replacing logging examples with a complete CRUD REST API implementation across two guides. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Pre-merge checks✅ Passed checks (3 passed)
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 |
Dangerous URL checkNo absolute URLs to prisma.io/docs found. |
Redirect checkThis PR probably requires the following redirects to be added to static/_redirects:
|
🍈 Lychee Link Check Report
📊 Results Overview
Errors per inputErrors in 800-guides/380-vercel-app-deployment.mdx
|
Deploying docs with
|
| Latest commit: |
8213233
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://55dc885a.docs-51g.pages.dev |
| Branch Preview URL: | https://deno-deploy-fix.docs-51g.pages.dev |
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: 1
🧹 Nitpick comments (6)
content/200-orm/200-prisma-client/500-deployment/301-edge/550-deploy-to-deno-deploy.mdx (4)
80-83: Consider combining the install commands.Running
deno installtwice is redundant. The second command with--allow-scriptswill perform the installation again. You can simplify this to a single command:deno install --allow-scriptsThe
--allow-scriptsflag permits lifecycle scripts (like Prisma's postinstall) to execute during installation.
186-193: Consider validating input fields on PATCH to prevent unintended field updates.Passing the raw request body directly to
prisma.task.update({ data: body })allows clients to potentially modify fields likeid,createdAt, orupdatedAt. While Prisma will ignore unknown fields, allowingidor timestamp overwrites could cause confusion or data integrity issues.For a documentation example, consider explicitly picking allowed fields:
🔎 Suggested improvement
// PATCH /tasks/:id - Update a task if (method === "PATCH") { const body = await request.json(); const task = await prisma.task.update({ where: { id }, - data: body, + data: { + title: body.title, + description: body.description, + completed: body.completed, + }, }); return json(task); }This teaches readers a safer pattern and avoids potential gotchas when they adapt the example.
196-199: DELETE returns 500 instead of 404 when task doesn't exist.The GET handler correctly returns 404 when a task isn't found, but DELETE (and PATCH) will throw a Prisma
P2025error that gets caught as a 500. For a documentation example teaching REST patterns, consistent 404 responses would be more instructive:🔎 Suggested improvement
// DELETE /tasks/:id - Delete a task if (method === "DELETE") { + const existing = await prisma.task.findUnique({ where: { id } }); + if (!existing) return json({ error: "Task not found" }, 404); await prisma.task.delete({ where: { id } }); return json({ message: "Task deleted" }); }Alternatively, you could catch the specific Prisma error code, but the extra query is simpler for documentation purposes.
278-283: Consider committingdeno.lockfor reproducible deployments.Ignoring
deno.lockmeans each deployment could resolve to different dependency versions if upstream packages change. For production deployments, committing the lock file ensures consistent builds:.env node_modules/ generated/ -deno.lockThis is a common best practice across package managers—similar to committing
package-lock.jsonoryarn.lockin Node.js projects.content/800-guides/400-deno-integration.mdx (2)
121-124: Same redundant install pattern as the deploy guide.As noted in the companion guide, these two commands can be combined into one:
-deno install -deno install --allow-scripts +deno install --allow-scripts
239-242: Install command should include--allow-scriptsfor consistency.Same as the deploy guide—consider adding
--allow-scriptsto ensure Prisma's lifecycle scripts execute:- - **Install command**: `deno install` + - **Install command**: `deno install --allow-scripts`
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
content/200-orm/200-prisma-client/500-deployment/301-edge/550-deploy-to-deno-deploy.mdxcontent/800-guides/400-deno-integration.mdx
⏰ 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: Check internal links
- GitHub Check: Cloudflare Pages
🔇 Additional comments (7)
content/200-orm/200-prisma-client/500-deployment/301-edge/550-deploy-to-deno-deploy.mdx (3)
89-108: LGTM!The Prisma schema correctly uses the new Prisma 7 patterns:
runtime = "deno"for Deno-compatible client generation, custom output directory, and the datasource without an inline URL (handled byprisma.config.ts). The Task model is well-structured with appropriate field types and defaults.
358-363: LGTM!The next steps section provides excellent follow-up guidance, pointing readers toward authentication patterns, validation libraries, Prisma extensions, and migration workflows. Good progression from the basic CRUD example to production-ready patterns.
300-303: Consider documenting why the deployment install differs from local setup.The local setup includes
deno install --allow-scriptsto enable Prisma's postinstall scripts, but the deployment config usesdeno installwithout it. This works because deployment handles Prisma client generation explicitly in the build step (deno run -A npm:prisma generate), not during install. Adding a note explaining this design—that deployment uses a prebuild approach to avoid relying on lifecycle scripts at install time—would reduce confusion for developers comparing local vs. deployed configurations.content/800-guides/400-deno-integration.mdx (4)
138-161: LGTM!The Prisma schema is well-structured with the
runtime = "deno"configuration, and the Log model with Level enum provides a good example of using enums in Prisma. The field types are appropriate for a logging use case.
167-175: LGTM!Good practice showing both
db:migratefor proper versioned migrations anddb:pushfor rapid prototyping. The--name initargument correctly passes through to the underlying Prisma command.
181-211: LGTM!The application code correctly demonstrates the PrismaPg adapter pattern with proper initialization and a practical logging use case. The favicon early-return at line 191-193 is a nice touch to avoid cluttering logs with browser requests.
273-285: LGTM!Good cross-referencing between the two guides, and the next steps provide a clear progression path for readers who want to expand their implementation.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.