docs: clarify Next.js instrumentation for multi-runtime compatibility#1033
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
dosubot
Bot
added
the
size:XS
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdded commented explanatory guidance around a dynamic import of Changes
Sequence Diagram(s)Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🔇 Additional comments (1)
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 |
Summary of ChangesHello @pieter-berkel, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request provides updated documentation to guide developers on how to prevent Node.js-specific imports from causing runtime errors in Edge environments. It introduces a new example within "apps/content/docs/adapters/next.md" that illustrates the use of "process.env.NEXT_RUNTIME" to conditionally load modules in "instrumentation.ts", thereby ensuring applications can operate seamlessly across both Node.js and Edge runtimes. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
pieter-berkel
changed the title
pieter-berkel
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| // Option 1: Simple global import (works with both Edge and Node.js runtimes) | ||
| await import('./lib/orpc.server') | ||
|
|
||
| // Option 2: Conditional import (only when using Node.js specific features) | ||
| // Use this if your server-side client uses Node.js APIs like fs, crypto, etc. | ||
| // if (process.env.NEXT_RUNTIME === "nodejs") { | ||
| // await import('./lib/orpc.server') | ||
| // } |
There was a problem hiding this comment.
Correct the “Option 1” guidance—it breaks on Edge.
A blanket import of ./lib/orpc.server still pulls in Node-only APIs and will crash on Edge, which is precisely what this PR set out to avoid. We need the docs to state that the unconditional import is only safe when the module is runtime-agnostic; otherwise the conditional path is required.
- // Option 1: Simple global import (works with both Edge and Node.js runtimes)
+ // Option 1: Simple global import (only safe if ./lib/orpc.server stays runtime-agnostic)
@@
- // Option 2: Conditional import (only when using Node.js specific features)
- // Use this if your server-side client uses Node.js APIs like fs, crypto, etc.
+ // Option 2: Conditional import (recommended when ./lib/orpc.server touches Node.js-only APIs)
+ // Use this when your server-side client relies on fs, crypto, or other Node.js-specific modules.📝 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.
| // Option 1: Simple global import (works with both Edge and Node.js runtimes) | |
| await import('./lib/orpc.server') | |
| // Option 2: Conditional import (only when using Node.js specific features) | |
| // Use this if your server-side client uses Node.js APIs like fs, crypto, etc. | |
| // if (process.env.NEXT_RUNTIME === "nodejs") { | |
| // await import('./lib/orpc.server') | |
| // } | |
| // Option 1: Simple global import (only safe if ./lib/orpc.server stays runtime-agnostic) | |
| await import('./lib/orpc.server') | |
| // Option 2: Conditional import (recommended when ./lib/orpc.server touches Node.js-only APIs) | |
| // Use this when your server-side client relies on fs, crypto, or other Node.js-specific modules. | |
| // if (process.env.NEXT_RUNTIME === "nodejs") { | |
| // await import('./lib/orpc.server') | |
| // } |
🤖 Prompt for AI Agents
In apps/content/docs/adapters/next.md around lines 167 to 174, the "Option 1"
guidance incorrectly suggests an unconditional import is safe; update the text
to state that an unconditional await import('./lib/orpc.server') is only safe if
that module is runtime-agnostic and contains no Node-only APIs, otherwise use
the conditional import pattern (e.g., checking NEXT_RUNTIME or similar) so Edge
runtimes are not forced to load Node-only code; change the wording to explicitly
warn that unconditional imports can pull Node-only APIs and crash on Edge, and
recommend using the conditional path when the module relies on Node-specific
features.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request aims to fix an issue with Node.js-specific imports in the Edge runtime, but the changes don't seem to fully implement the described fix.
-
Code vs. Documentation: The PR description states that
instrumentation.tswas updated to conditionally import./lib/orpc.server. However, the only file changed is the documentation fileapps/content/docs/adapters/next.md. The actual implementation fileplaygrounds/next/src/instrumentation.tsremains unchanged, meaning the bug described is likely not fixed in the example code. -
Inconsistent Documentation: The documentation for
instrumentation.tsis updated inapps/content/docs/adapters/next.md, but an identical code snippet inapps/content/docs/best-practices/optimize-ssr.mdhas not been updated, leading to inconsistency across the documentation. -
Clarity of Documentation: The new documentation in
next.mdcould be clearer in explaining that the two options presented are mutually exclusive. I've added a specific suggestion to improve this.
Please consider applying the actual code fix to instrumentation.ts and updating all relevant documentation files for consistency.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/content/docs/adapters/next.md(1 hunks)apps/content/docs/best-practices/optimize-ssr.md(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/content/docs/adapters/next.md
dinwwwh
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
More templates
@orpc/arktype
@orpc/client
@orpc/contract
@orpc/experimental-durable-iterator
@orpc/hey-api
@orpc/interop
@orpc/json-schema
@orpc/nest
@orpc/openapi
@orpc/openapi-client
@orpc/otel
@orpc/react
@orpc/react-query
@orpc/experimental-react-swr
@orpc/server
@orpc/shared
@orpc/solid-query
@orpc/standard-server
@orpc/standard-server-aws-lambda
@orpc/standard-server-fetch
@orpc/standard-server-node
@orpc/standard-server-peer
@orpc/svelte-query
@orpc/tanstack-query
@orpc/trpc
@orpc/valibot
@orpc/vue-colada
@orpc/vue-query
@orpc/zod
commit: |
2 participants
This PR fixes an issue where Node.js-specific imports were being executed in the Edge runtime, causing runtime errors (since the Edge runtime does not support Node.js APIs such as fs, crypto, etc.).
What caused the issue
instrumentation.ts was always importing ./lib/orpc.server, regardless of runtime.
This worked fine in Node.js, but failed in the Edge runtime since it attempts to load modules that rely on Node-specific APIs.
What changed
Updated instrumentation.ts to check the NEXT_RUNTIME environment variable before importing.
Ensures that ./lib/orpc.server only loads when NEXT_RUNTIME === "nodejs".
This allows the same codebase to work seamlessly across both Node.js and Edge runtimes.
Why this fix
Prevents unnecessary runtime errors in Edge.
Keeps Node.js-specific logic scoped to the right runtime.
Improves compatibility and reliability when deploying across different environments.
Summary by CodeRabbit