feat(vite): experimental rsc plugin support #3919
Conversation
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
|
@hi-ogawa is attempting to deploy a commit to the Nitro Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughAdds a Vite + React Server Components (RSC) example and updates Nitro's Vite integration: introduces configurable experimental Vite services, exposes an environment runner import helper, adjusts Rollup service input mapping, and wires example app files for RSC streaming, SSR, and server actions. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
The example has its own tsconfig with Bundler moduleResolution and @vitejs/plugin-rsc/types. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
|
It looks like vite-rsc example is failing with rollup. I'm not sure how to test this locally. 🤔 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Fix all issues with AI Agents 🤖
In @examples/vite-rsc/package.json:
- Around line 19-23: The package.json uses floating tags causing
non-reproducible installs; replace the non-pinned entries—specifically
"@vitejs/plugin-react", "nitro", and "vite"—that are set to "latest" or "beta"
with explicit, tested version numbers (e.g., exact semver like "x.y.z") so
installs are deterministic; keep the temporary prerelease URL for
"@vitejs/plugin-rsc" as-is per the PR note or update it later when an official
release is available.
In @examples/vite-rsc/README.md:
- Line 3: Replace the bare URL in the README line "Copied from
https://github.com/vitejs/vite-plugin-react/tree/main/packages/plugin-rsc/examples/starter"
with a proper Markdown link or angle-bracketed URL; for example change it to
either a linked text like "Copied from [vite-plugin-react example
starter](https://github.com/vitejs/vite-plugin-react/tree/main/packages/plugin-rsc/examples/starter)"
or to an explicit URL in angle brackets
"<https://github.com/vitejs/vite-plugin-react/tree/main/packages/plugin-rsc/examples/starter>"
so it renders/accesses correctly.
In @examples/vite-rsc/src/framework/entry.browser.tsx:
- Around line 51-64: The handler passed to setServerCallback assumes
payload.returnValue is always present and uses a non-null assertion; replace
that with a guard: after createFromFetch resolves, check if payload.returnValue
is undefined/null and if so log or throw a descriptive error (including payload
or status) before accessing its fields, then proceed to destructure { ok, data }
from payload.returnValue only when present; update the setServerCallback
callback to return or throw appropriately so callers get a clear failure instead
of an unexpected runtime crash.
In @examples/vite-rsc/src/framework/error-boundary.tsx:
- Around line 29-31: The reset method sets this.setState({ error: null }) but
the component state declares error?: Error (Error | undefined); change the reset
implementation in the ErrorBoundary (reset method) to set error to undefined
instead of null to match the typed optional property and avoid a type mismatch.
In @examples/vite-rsc/src/root.tsx:
- Around line 57-68: The anchor elements that open demo pages in a new tab (the
<a> tags with href="?__rsc" and href="?__nojs" in root.tsx) are missing
rel="noopener"; update those anchor elements to include rel="noopener" alongside
target="_blank" to satisfy the linter and prevent the new page from accessing
window.opener.
- Around line 28-36: External anchor elements that open in a new tab (the <a>
wrapping viteLogo and the <a> wrapping reactLogo which both use target="_blank")
lack rel="noopener" and can expose window.opener to the opened page; update
those anchor tags to include rel="noopener" (or rel="noopener noreferrer")
alongside target="_blank" to mitigate the security risk.
🧹 Nitpick comments (4)
src/runtime/internal/vite/node-runner.mjs (1)
13-24: Add JSDoc documentation and consider defensive input validation.This global function is already correctly documented via the GitHub link in the comment. The function signature and behavior align with the RSC plugin integration pattern.
Two optional improvements:
Documentation: Add JSDoc comments explaining the purpose, parameters, return value, and error conditions for clarity to future maintainers.
Defensive validation (optional): While
envsis internally managed and the function is only called by the RSC plugin, adding basic type checks on parameters would provide robustness:
- Validate
environmentNameis a non-empty string- Validate
idis a non-empty stringThe interface contract referenced in the code comment is the authoritative specification.
Suggested JSDoc and optional validation
// define __VITE_ENVIRONMENT_RUNNER_IMPORT__ for RSC support // https://github.com/vitejs/vite-plugin-react/blob/main/packages/plugin-rsc/README.md#__vite_environment_runner_import__ +/** + * Global import function for React Server Components support. + * Allows the RSC plugin to dynamically import modules from registered Vite environments. + * + * @param {string} environmentName - Name of the registered Vite environment + * @param {string} id - Module identifier to import + * @returns {Promise<any>} The imported module + * @throws {Error} If the environment is not registered + */ globalThis.__VITE_ENVIRONMENT_RUNNER_IMPORT__ = async function ( environmentName, id ) { const env = envs[environmentName]; if (!env) { throw new Error(`Vite environment "${environmentName}" is not registered`); } return env.runner.import(id); };examples/vite-rsc/src/framework/error-boundary.tsx (1)
64-67: Optional: Remove unnecessary property check.The check
"message" in props.erroris unnecessary sinceprops.erroris typed asError, and all Error objects have amessageproperty. You can simplify toprops.error.message.🔎 Proposed simplification
- Error:{" "} - {import.meta.env.DEV && "message" in props.error - ? props.error.message - : "(Unknown)"} + Error: {import.meta.env.DEV ? props.error.message : "(Unknown)"}examples/vite-rsc/src/framework/entry.rsc.tsx (1)
62-71: Consider logging the suppressed error for debugging.The catch block silently discards the error before returning a 500 response. For debugging purposes, consider logging the error (e.g., to
console.error) even though the response correctly indicates failure.🔎 Proposed improvement
try { const result = await decodedAction(); formState = await decodeFormState(result, formData); - } catch { + } catch (error) { + console.error("Server action failed:", error); return new Response("Internal Server Error: server action failed", { status: 500, }); }examples/vite-rsc/src/framework/entry.ssr.tsx (1)
43-60: Consider logging SSR errors for debugging.The catch block silently falls back to CSR without logging the SSR failure. This is reasonable for production resilience, but logging the error would help during development.
🔎 Proposed improvement
- } catch { + } catch (error) { + console.error("SSR render failed, falling back to CSR:", error); // fallback to render an empty shell and run pure CSR on browser, // which can replay server component error and trigger error boundary. status = 500;
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
examples/vite-rsc/public/vite.svgis excluded by!**/*.svgexamples/vite-rsc/src/assets/react.svgis excluded by!**/*.svgpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (19)
examples/vite-rsc/.gitignoreexamples/vite-rsc/README.mdexamples/vite-rsc/package.jsonexamples/vite-rsc/src/action.tsxexamples/vite-rsc/src/client.tsxexamples/vite-rsc/src/framework/entry.browser.tsxexamples/vite-rsc/src/framework/entry.rsc.tsxexamples/vite-rsc/src/framework/entry.ssr.tsxexamples/vite-rsc/src/framework/error-boundary.tsxexamples/vite-rsc/src/framework/request.tsxexamples/vite-rsc/src/index.cssexamples/vite-rsc/src/root.tsxexamples/vite-rsc/tsconfig.jsonexamples/vite-rsc/vite.config.tssrc/build/vite/env.tssrc/build/vite/plugin.tssrc/build/vite/types.tssrc/runtime/internal/vite/node-runner.mjstsconfig.json
🧰 Additional context used
📓 Path-based instructions (4)
src/**/*.{ts,js,tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,js,tsx,jsx}: Usepathefor cross-platform path operations instead of Node.jsnode:path
Use ESM and modern JavaScript
Do not add comments explaining what the line does unless prompted
Files:
src/build/vite/env.tssrc/build/vite/types.tssrc/build/vite/plugin.ts
src/{build,dev,runner,cli}/**/*.{ts,js}
📄 CodeRabbit inference engine (AGENTS.md)
Use
consolafor logging in build/dev code; usenitro.loggerwhen available
Files:
src/build/vite/env.tssrc/build/vite/types.tssrc/build/vite/plugin.ts
src/**/*.{ts,js}
📄 CodeRabbit inference engine (AGENTS.md)
Use
unstoragefor storage abstraction
Files:
src/build/vite/env.tssrc/build/vite/types.tssrc/build/vite/plugin.ts
src/build/**/*.{ts,js}
📄 CodeRabbit inference engine (AGENTS.md)
Virtual modules must be registered in
src/build/virtual.ts
Files:
src/build/vite/env.tssrc/build/vite/types.tssrc/build/vite/plugin.ts
🧠 Learnings (6)
📚 Learning: 2025-12-24T11:45:17.435Z
Learnt from: CR
Repo: nitrojs/nitro PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-24T11:45:17.435Z
Learning: Applies to src/**/*.{ts,js,tsx,jsx} : Use ESM and modern JavaScript
Applied to files:
examples/vite-rsc/tsconfig.json
📚 Learning: 2025-12-24T11:45:17.435Z
Learnt from: CR
Repo: nitrojs/nitro PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-24T11:45:17.435Z
Learning: Applies to src/runtime/**/*.{ts,js} : Keep runtime code minimal and side-effect free to reduce bundle size
Applied to files:
examples/vite-rsc/tsconfig.json
📚 Learning: 2025-12-24T11:45:17.435Z
Learnt from: CR
Repo: nitrojs/nitro PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-24T11:45:17.435Z
Learning: Applies to src/config/**/*.{ts,js} : Use `defu` for deep object merging and config defaults
Applied to files:
examples/vite-rsc/tsconfig.json
📚 Learning: 2025-12-24T11:45:17.435Z
Learnt from: CR
Repo: nitrojs/nitro PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-24T11:45:17.435Z
Learning: Applies to src/build/**/*.{ts,js} : Virtual modules must be registered in `src/build/virtual.ts`
Applied to files:
examples/vite-rsc/tsconfig.jsontsconfig.json
📚 Learning: 2025-12-24T11:45:17.435Z
Learnt from: CR
Repo: nitrojs/nitro PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-24T11:45:17.435Z
Learning: Applies to src/cli/commands/**/*.{ts,js} : CLI commands are in `src/cli/commands/` — each file exports a command definition
Applied to files:
tsconfig.json
📚 Learning: 2025-12-24T11:45:17.435Z
Learnt from: CR
Repo: nitrojs/nitro PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-24T11:45:17.435Z
Learning: Update types and JSDoc for API changes
Applied to files:
src/build/vite/types.ts
🧬 Code graph analysis (6)
examples/vite-rsc/src/framework/entry.ssr.tsx (1)
src/runtime/internal/vite/node-runner.mjs (1)
payload(144-144)
examples/vite-rsc/src/framework/entry.rsc.tsx (2)
examples/vite-rsc/src/framework/request.tsx (1)
parseRenderRequest(34-58)examples/vite-rsc/src/root.tsx (1)
Root(7-22)
examples/vite-rsc/src/root.tsx (2)
examples/vite-rsc/src/client.tsx (1)
ClientCounter(5-13)examples/vite-rsc/src/action.tsx (2)
updateServerCounter(9-11)getServerCounter(5-7)
examples/vite-rsc/src/framework/entry.browser.tsx (4)
examples/vite-rsc/src/framework/entry.rsc.tsx (1)
RscPayload(15-24)src/runtime/internal/vite/node-runner.mjs (1)
payload(144-144)examples/vite-rsc/src/framework/request.tsx (1)
createRscRenderRequest(17-32)examples/vite-rsc/src/framework/error-boundary.tsx (1)
GlobalErrorBoundary(6-12)
src/build/vite/types.ts (1)
src/vite.ts (1)
ServiceConfig(3-3)
examples/vite-rsc/vite.config.ts (1)
src/build/vite/plugin.ts (1)
nitro(39-56)
🪛 Biome (2.1.2)
examples/vite-rsc/src/root.tsx
[error] 28-28: Avoid using target="_blank" without rel="noopener" or rel="noreferrer".
Opening external links in new tabs without rel="noopener" is a security risk. See the explanation for more details.
Safe fix: Add the rel="noopener" attribute.
(lint/security/noBlankTarget)
[error] 33-33: Avoid using target="_blank" without rel="noopener" or rel="noreferrer".
Opening external links in new tabs without rel="noopener" is a security risk. See the explanation for more details.
Safe fix: Add the rel="noopener" attribute.
(lint/security/noBlankTarget)
[error] 57-57: Avoid using target="_blank" without rel="noopener" or rel="noreferrer".
Opening external links in new tabs without rel="noopener" is a security risk. See the explanation for more details.
Safe fix: Add the rel="noopener" attribute.
(lint/security/noBlankTarget)
[error] 64-64: Avoid using target="_blank" without rel="noopener" or rel="noreferrer".
Opening external links in new tabs without rel="noopener" is a security risk. See the explanation for more details.
Safe fix: Add the rel="noopener" attribute.
(lint/security/noBlankTarget)
🪛 markdownlint-cli2 (0.18.1)
examples/vite-rsc/README.md
3-3: Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (23)
examples/vite-rsc/.gitignore (1)
1-2: LGTM — standard ignore patterns in place.The core patterns (
node_modules,dist) are appropriate for a Node.js/Vite example. If you'd like a more comprehensive.gitignore, consider adding patterns for environment files, OS artifacts, and logs (e.g.,.env.local,.DS_Store,*.log), but the current minimalist approach is sufficient for a demo.tsconfig.json (1)
16-16: LGTM! Appropriate exclusion for the new example.Excluding the new Vite RSC example from the root TypeScript compilation allows it to use its own isolated configuration, consistent with other examples in the repository.
examples/vite-rsc/tsconfig.json (1)
1-18: LGTM! Modern TypeScript configuration for Vite + RSC.The configuration appropriately uses
"Bundler"moduleResolution, strict type-checking, and includes the necessary types for Vite client and RSC plugin support. This aligns well with the modern Vite + React 19 + RSC setup.examples/vite-rsc/src/index.css (1)
1-112: LGTM! Standard starter template styling.The CSS provides appropriate theme handling, typography, and layout for the example project.
examples/vite-rsc/src/framework/error-boundary.tsx (1)
5-12: LGTM!The GlobalErrorBoundary wrapper is correctly structured and provides a clean API for the error boundary functionality.
examples/vite-rsc/src/framework/request.tsx (3)
1-15: LGTM!The request parsing conventions are clearly documented, and the
RenderRequesttype provides a clean abstraction for routing RSC/SSR requests.
17-32: LGTM!The function correctly constructs RSC requests with appropriate URL suffix, headers, and HTTP method based on whether it's an action.
34-58: LGTM!The parsing logic correctly identifies RSC requests, validates action requirements, and normalizes the URL by removing the suffix. The error handling for missing action IDs is appropriate.
examples/vite-rsc/src/client.tsx (1)
1-13: LGTM!The ClientCounter component is correctly implemented with the
"use client"directive and uses the updater function form ofsetCountfor safe state updates.src/build/vite/plugin.ts (1)
285-291: LGTM! The experimental services configuration is properly typed.The initialization correctly propagates optional Vite services from the plugin config into the context. When services are not provided, the optional chaining returns
undefined, and the spread operator safely produces an empty object that matches the requiredRecord<string, ServiceConfig>type. This aligns with the PR's objective to supportexperimental.vite.servicesconfiguration.src/build/vite/env.ts (1)
63-93: The named input structure is valid and Vite handles it correctly, but the downstream tracking differs from the claim.The change to
input: { index: serviceConfig.entry }creates a valid Vite rollupOptions configuration. ThegetEntryfunction does support object inputs with an "index" property (line 444-445 of plugin.ts), but service environments don't route throughgetEntry—instead, service entry points are directly captured via thectx._entryPoints[name]tracking mechanism during the buildEnd hook (plugin.ts:193) and used downstream in prod.ts:166. The output directory structure (buildDir/vite/services/{name}) correctly isolates each service build, and the entry point resolution via_entryPointsensures proper output naming.src/build/vite/types.ts (2)
37-42: LGTM!The type changes appropriately support the new RSC plugin integration:
- Making
serverReloadoptional aligns with its documented default oftrue- The new
servicesproperty provides a clean API for registering additional Vite environment services (SSR, RSC entries)
47-49: LGTM!The
ServiceConfiginterface is appropriately minimal for its purpose.examples/vite-rsc/vite.config.ts (2)
1-4: LGTM!Import structure is clean and appropriately organized.
6-26: Configuration looks correct; verify prerelease compatibility.The plugin configuration properly demonstrates the new
experimental.vite.servicesAPI with SSR and RSC entries. TheserverHandler: falseoption for the RSC plugin is appropriate since Nitro handles server routing.Note that this example uses prerelease packages (
@vitejs/plugin-rscfrom pkg.pr.new andvitebeta). Ensure these are updated to stable releases when available.examples/vite-rsc/src/root.tsx (1)
43-45: Clarify: async function in JSX is valid for React Server Components.The
getServerCounter()call works correctly here because this is a Server Component, where React supports async functions and will await the promise during rendering.examples/vite-rsc/src/action.tsx (1)
1-11: LGTM for demo purposes.The in-memory server counter is appropriate for demonstrating RSC server actions. The
"use server"directive and async function signatures are correct.Note: In a production application, this state would be lost on server restart and wouldn't work correctly with multiple server instances. Consider adding a brief comment indicating this is demo-only if helpful for users referencing this example.
examples/vite-rsc/src/framework/entry.browser.tsx (1)
91-136: Navigation listener implementation is well-structured.The
listenNavigationhelper properly interceptspopstate,pushState,replaceState, and link clicks with appropriate checks (same origin, no modifier keys, etc.). The cleanup function correctly restores the original history methods.examples/vite-rsc/src/framework/entry.rsc.tsx (3)
15-24: LGTM!The
RscPayloadtype is well-documented and covers the necessary cases for RSC streaming, server action return values, and form state for progressive enhancement.
88-95: Response status defaults to 200 whenactionStatusis undefined.When
actionStatusis undefined (non-error case), the Response constructor will use the default status of 200, which is correct behavior. The code works as intended.
101-108: LGTM!The SSR delegation pattern using
import.meta.viteRsc.loadModulecleanly separates RSC and SSR concerns while allowing customization for different runtimes (as noted in the comment about Cloudflare service bindings).examples/vite-rsc/src/framework/entry.ssr.tsx (2)
16-28: LGTM!The stream tee pattern correctly duplicates the RSC stream for parallel SSR rendering and client hydration payload injection. The lazy initialization of
payloadvia??=ensures deserialization happens only once within the React SSR context.
76-82: LGTM!The default export properly delegates to the RSC entry module via
import.meta.viteRsc.loadModule, maintaining clean separation between SSR and RSC entry points.
| "@vitejs/plugin-react": "latest", | ||
| "@vitejs/plugin-rsc": "https://pkg.pr.new/@vitejs/plugin-rsc@687458d", | ||
| "nitro": "latest", | ||
| "rsc-html-stream": "^0.0.7", | ||
| "vite": "beta" |
There was a problem hiding this comment.
Pin dependency versions for reproducibility.
Using "latest" and "beta" for dependencies creates non-reproducible builds. Different developers and CI runs could pull different versions, leading to inconsistent behavior and potential breakage.
For example projects especially, pinned or ranged versions ensure users get a known-working configuration.
🔎 Recommended changes
- "@vitejs/plugin-react": "latest",
+ "@vitejs/plugin-react": "^4.3.4",
"@vitejs/plugin-rsc": "https://pkg.pr.new/@vitejs/plugin-rsc@687458d",
- "nitro": "latest",
+ "nitro": "^2.10.4",
"rsc-html-stream": "^0.0.7",
- "vite": "beta"
+ "vite": "^6.0.0-beta.0"Note: Replace with actual target versions. The prerelease URL at line 20 is acknowledged as temporary in the PR description and can be addressed separately when the official release is available.
📝 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.
| "@vitejs/plugin-react": "latest", | |
| "@vitejs/plugin-rsc": "https://pkg.pr.new/@vitejs/plugin-rsc@687458d", | |
| "nitro": "latest", | |
| "rsc-html-stream": "^0.0.7", | |
| "vite": "beta" | |
| "@vitejs/plugin-react": "^4.3.4", | |
| "@vitejs/plugin-rsc": "https://pkg.pr.new/@vitejs/plugin-rsc@687458d", | |
| "nitro": "^2.10.4", | |
| "rsc-html-stream": "^0.0.7", | |
| "vite": "^6.0.0-beta.0" |
🤖 Prompt for AI Agents
In @examples/vite-rsc/package.json around lines 19-23, The package.json uses
floating tags causing non-reproducible installs; replace the non-pinned
entries—specifically "@vitejs/plugin-react", "nitro", and "vite"—that are set to
"latest" or "beta" with explicit, tested version numbers (e.g., exact semver
like "x.y.z") so installs are deterministic; keep the temporary prerelease URL
for "@vitejs/plugin-rsc" as-is per the PR note or update it later when an
official release is available.
| consumer: "server", | ||
| build: { | ||
| rollupOptions: { input: serviceConfig.entry }, | ||
| rollupOptions: { input: { index: serviceConfig.entry } }, |
There was a problem hiding this comment.
it would be nice if plugins support both string and { index: string } formats..
There was a problem hiding this comment.
This should be now possible with vitejs/vite-plugin-react#1040, but I don't think enforcing string form on Nitro is a good direction (though I'm not sure whether hard-coding index is good either, so maybe nitro specific entry name?).
If Nitro has need for this to be single entry, then RSC plugin and rsc example can adjust it, but not sure if framework can adjust it easily. Please let me know if reverting it back { input: serviceConfig.entry } is necessary for Nitro.
pi0
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
Fix all issues with AI Agents 🤖
In @examples/vite-rsc/app/action.tsx:
- Around line 1-11: The module-scoped mutable serverCounter causes shared state
and race conditions; remove the global serverCounter and either (A) add a clear
warning comment above getServerCounter/updateServerCounter that explains
module-scoped state is shared across requests (minimal demo fix), or (B) change
the API to use request-scoped keys and a Map-based store: replace serverCounter
with a sessionCounters Map and update function signatures to
getServerCounter(sessionId: string) and updateServerCounter(sessionId: string,
change: number) so each caller provides a session/user key, or instead persist
counters to an external store (DB/Redis) for true isolation; update all callers
to pass the sessionId and document the new behavior in comments.
In @src/runtime/internal/vite/node-runner.mjs:
- Around line 158-172: The documentation URL for
__VITE_ENVIRONMENT_RUNNER_IMPORT__ is invalid; update the comment and
verification around this function by either replacing the broken link with the
correct upstream reference or removing the dead URL and adding a short note that
the signature was derived from source inspection; specifically, confirm that
globalThis.__VITE_ENVIRONMENT_RUNNER_IMPORT__ (the async function that looks up
envs[environmentName] and calls env.runner.import(id)) matches the plugin's
expected API by linking to the correct @vitejs/plugin-rsc source/PR or adding a
one-line TODO referencing the exact commit/PR used for verification, and if the
plugin API differs, adjust the function to call the correct method name or
parameter shape on env.runner (e.g., env.runner.import vs env.runner.load) to
match the inspected plugin implementation.
♻️ Duplicate comments (5)
examples/vite-rsc/app/framework/error-boundary.tsx (1)
29-31: Type inconsistency: Useundefinedinstead ofnull.This issue was previously flagged. The state type declares
error?: Error(which meansError | undefined), but the reset method setserror: null. For type consistency, useundefinedto match the optional property type.🔎 Proposed fix
reset = () => { - this.setState({ error: null }); + this.setState({ error: undefined }); };examples/vite-rsc/app/framework/entry.browser.tsx (2)
15-47: Potential race:setPayloadmay be called before it's assigned.The
setPayloadvariable is assigned insideuseEffect, butfetchRscPayload()can be invoked before the effect runs (e.g., fromlistenNavigationduring initial render). This could cause a runtime error whensetPayload(payload)is called on line 46 whilesetPayloadis still undefined.Consider initializing with a no-op or queueing updates until the setter is available.
51-64: Non-null assertion onreturnValuemay throw at runtime.Line 61 assumes
payload.returnValueis always present after a server callback. If the server response is malformed or an unexpected error occurs, this will throw. Consider adding a defensive guard.examples/vite-rsc/app/root.tsx (2)
29-41: Addrel="noopener"to external links withtarget="_blank".External links opening in new tabs without
rel="noopener"allow the opened page to accesswindow.opener, which is a security risk.🔎 Proposed fix
- <a href="https://vite.dev" target="_blank"> + <a href="https://vite.dev" target="_blank" rel="noopener"> <img src={viteLogo} className="logo" alt="Vite logo" /> </a> <a href="https://react.dev/reference/rsc/server-components" target="_blank" + rel="noopener" > <img src={reactLogo} className="logo react" alt="React logo" /> </a> - <a href="https://v3.nitro.build" target="_blank"> + <a href="https://v3.nitro.build" target="_blank" rel="noopener"> <img src={nitroLogo} className="logo" alt="Nitro logo" /> </a>
67-73: Addrel="noopener"to the?__nojsdemo link.Even for same-origin links, adding
rel="noopener"is good practice for consistency and to satisfy linter rules.🔎 Proposed fix
- <a href="?__nojs" target="_blank"> + <a href="?__nojs" target="_blank" rel="noopener"> <code>?__nojs</code> </a>{" "}
🧹 Nitpick comments (3)
examples/vite-rsc/app/framework/entry.browser.tsx (1)
74-80: Consider adding a comment explaining the__NO_HYDRATEflag.The conditional on line 74 checks for
__NO_HYDRATEinglobalThis, which is set in the SSR fallback path. A brief comment would help future maintainers understand this CSR-only fallback mechanism.examples/vite-rsc/app/framework/request.tsx (1)
50-56:isActionmay be misleading for non-RSC POST requests.For non-RSC requests (the
elsebranch),isActionis set totruefor any POST request. However, theactionIdfield won't be populated since it's only extracted from RSC requests. This could lead to confusion if a consumer checksisActionwithout also checkingisRsc.Consider either:
- Setting
isAction: falsefor non-RSC requests (since they're handled differently)- Adding a comment clarifying the semantics
🔎 Option 1: Set isAction false for non-RSC
} else { return { isRsc: false, - isAction, + isAction: false, // Server actions only supported via RSC endpoint request, url, }; }examples/vite-rsc/app/framework/entry.rsc.tsx (1)
68-74: Empty catch block loses error context for debugging.The catch block on line 68 returns a generic 500 response without logging or including any error details. While this is acceptable for production security, it may make debugging difficult during development.
🔎 Proposed improvement
- } catch { + } catch (error) { // there's no single general obvious way to surface this error, // so explicitly return classic 500 response. + if (import.meta.env?.DEV) { + console.error("Server action failed:", error); + } return new Response("Internal Server Error: server action failed", { status: 500, }); }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
examples/vite-rsc/app/assets/nitro.svgis excluded by!**/*.svgexamples/vite-rsc/app/assets/react.svgis excluded by!**/*.svgexamples/vite-rsc/app/assets/vite.svgis excluded by!**/*.svg
📒 Files selected for processing (12)
examples/vite-rsc/app/action.tsxexamples/vite-rsc/app/client.tsxexamples/vite-rsc/app/framework/entry.browser.tsxexamples/vite-rsc/app/framework/entry.rsc.tsxexamples/vite-rsc/app/framework/entry.ssr.tsxexamples/vite-rsc/app/framework/error-boundary.tsxexamples/vite-rsc/app/framework/request.tsxexamples/vite-rsc/app/index.cssexamples/vite-rsc/app/root.tsxexamples/vite-rsc/tsconfig.jsonexamples/vite-rsc/vite.config.tssrc/runtime/internal/vite/node-runner.mjs
🚧 Files skipped from review as they are similar to previous changes (1)
- examples/vite-rsc/tsconfig.json
🧰 Additional context used
🧬 Code graph analysis (5)
examples/vite-rsc/vite.config.ts (1)
src/build/vite/plugin.ts (1)
nitro(39-56)
examples/vite-rsc/app/framework/entry.rsc.tsx (2)
examples/vite-rsc/app/framework/request.tsx (1)
parseRenderRequest(34-58)examples/vite-rsc/app/root.tsx (1)
Root(8-23)
examples/vite-rsc/app/framework/entry.ssr.tsx (1)
examples/vite-rsc/app/framework/entry.rsc.tsx (1)
RscPayload(15-26)
examples/vite-rsc/app/framework/request.tsx (2)
src/runtime/internal/vite/node-runner.mjs (1)
url(309-309)src/runtime/internal/route-rules.ts (1)
headers(14-19)
examples/vite-rsc/app/framework/entry.browser.tsx (3)
examples/vite-rsc/app/framework/entry.rsc.tsx (1)
RscPayload(15-26)examples/vite-rsc/app/framework/request.tsx (1)
createRscRenderRequest(17-32)examples/vite-rsc/app/framework/error-boundary.tsx (1)
GlobalErrorBoundary(6-12)
🪛 Biome (2.1.2)
examples/vite-rsc/app/root.tsx
[error] 29-29: Avoid using target="_blank" without rel="noopener" or rel="noreferrer".
Opening external links in new tabs without rel="noopener" is a security risk. See the explanation for more details.
Safe fix: Add the rel="noopener" attribute.
(lint/security/noBlankTarget)
[error] 34-34: Avoid using target="_blank" without rel="noopener" or rel="noreferrer".
Opening external links in new tabs without rel="noopener" is a security risk. See the explanation for more details.
Safe fix: Add the rel="noopener" attribute.
(lint/security/noBlankTarget)
[error] 39-39: Avoid using target="_blank" without rel="noopener" or rel="noreferrer".
Opening external links in new tabs without rel="noopener" is a security risk. See the explanation for more details.
Safe fix: Add the rel="noopener" attribute.
(lint/security/noBlankTarget)
[error] 69-69: Avoid using target="_blank" without rel="noopener" or rel="noreferrer".
Opening external links in new tabs without rel="noopener" is a security risk. See the explanation for more details.
Safe fix: Add the rel="noopener" attribute.
(lint/security/noBlankTarget)
🔇 Additional comments (15)
examples/vite-rsc/vite.config.ts (3)
9-18: LGTM! Nitro services configuration is well-structured.The experimental Vite services configuration correctly defines separate entries for SSR and RSC rendering, which aligns with the PR's objective to support @vitejs/plugin-rsc integration.
23-31: LGTM! Client environment configuration correctly addresses RSC plugin requirements.The explicit
rollupOptions.input.indexconfiguration aligns with the RSC plugin's expectations, as noted in the PR objectives.
19-19: Configuration is correct.The
serverHandler: falseoption disables the plugin's built-in dev server middleware, allowing Nitro to handle server operations as intended.examples/vite-rsc/app/framework/error-boundary.tsx (2)
5-27: LGTM! Error boundary implementation follows React best practices.The ErrorBoundary class correctly implements the standard React error boundary pattern using
getDerivedStateFromErrorand conditional rendering. The GlobalErrorBoundary wrapper provides a clean API for consumers.Also applies to: 33-40
44-81: LGTM! Error page implementation with good security practices.The DefaultGlobalErrorPage correctly:
- Shows error details only in DEV mode (good security practice)
- Uses
startTransitionappropriately for the reset action, which aligns with React 19's concurrent rendering features- Provides accessible recovery UI with inline styles that work independently of app styles
examples/vite-rsc/app/client.tsx (1)
1-13: LGTM! Clean client component implementation.The ClientCounter component correctly uses the "use client" directive and follows React best practices with
useStateand functional state updates.examples/vite-rsc/app/framework/entry.browser.tsx (1)
91-136: LGTM — navigation interception is well-structured.The
listenNavigationhelper correctly:
- Patches
pushState/replaceStateto trigger navigation callbacks- Filters link clicks appropriately (same-origin, left-click, no modifiers)
- Returns a cleanup function to restore original behavior
examples/vite-rsc/app/root.tsx (1)
47-51: LGTM — Server action form binding is correct.The form correctly binds
updateServerCounterwith a pre-applied argument using.bind(null, 1), which is the idiomatic pattern for React 19 server actions with progressive enhancement support.examples/vite-rsc/app/framework/request.tsx (1)
17-32: LGTM — Request construction is correct.The
createRscRenderRequestfunction properly:
- Appends the RSC URL suffix
- Sets the action header when provided
- Uses POST for actions and GET for navigation
examples/vite-rsc/app/framework/entry.rsc.tsx (3)
30-33: LGTM — Request parsing and normalization.The handler correctly parses the render request and reassigns to the normalized request object for consistent downstream handling.
78-99: LGTM — RSC payload construction and response handling.The RSC payload is correctly constructed after action handling to reflect updated state, and the response properly sets the
text/x-componentcontent type for RSC streams.
51-58: The concern in this review comment is not valid. ThedecodeReplyfunction has a documented return type ofPromise<unknown[]>, which means it always returns an array. Therefore,argsis guaranteed to be an array when passed toaction.apply(null, args), and the code is correct as written.The
eslint-disable-next-line prefer-spreadcomment indicates the developers made a deliberate choice to useapply()instead of spread syntax, which is acceptable. No changes are needed.examples/vite-rsc/app/framework/entry.ssr.tsx (3)
31-37: Clever lazy initialization pattern for SSR.The
payload ??= createFromReadableStream(...)pattern ensures deserialization only happens once and is kicked off inside the React render context, which is necessary for preloading to work correctly. This is a well-designed approach.
54-71: LGTM — SSR fallback with CSR recovery.The error fallback correctly:
- Sets
status: 500for proper HTTP semantics- Renders a minimal shell with
<noscript>message- Sets
__NO_HYDRATE=1to signal the browser entry to usecreateRootinstead ofhydrateRoot- Still injects bootstrap script for CSR error boundary recovery
This allows client-side error boundaries to catch and display the server error gracefully.
8-15: LGTM — Clean delegation to RSC entry.The default export's fetch method cleanly loads and delegates to the RSC entry module, maintaining proper separation between SSR and RSC concerns.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
test/examples.test.ts (1)
21-21: Consider adding a comment to explain the conditional skip.The logic correctly skips the RSC test with Rollup (where it fails) but allows it with Rolldown. Adding a brief comment would help future maintainers understand why this conditional exists without checking the git history.
🔎 Suggested addition
+// Skip vite-rsc test with Rollup due to incompatibility; works with Rolldown const skipProd = new Set<string>(isRolldown ? [] : ["vite-rsc"]);
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/examples.test.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-24T11:45:17.435Z
Learnt from: CR
Repo: nitrojs/nitro PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-24T11:45:17.435Z
Learning: Applies to src/runtime/**/*.{ts,js} : Keep runtime code minimal and side-effect free to reduce bundle size
Applied to files:
test/examples.test.ts
📚 Learning: 2025-12-24T11:45:17.435Z
Learnt from: CR
Repo: nitrojs/nitro PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-24T11:45:17.435Z
Learning: Applies to src/runtime/**/*.{ts,js} : Runtime code in `src/runtime/` must be runtime-agnostic: do not use Node.js-specific APIs unless behind runtime checks
Applied to files:
test/examples.test.ts
⏰ 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). (3)
- GitHub Check: tests-rolldown (windows-latest)
- GitHub Check: tests-rollup (ubuntu-latest)
- GitHub Check: tests-rollup (windows-latest)
🔇 Additional comments (2)
test/examples.test.ts (2)
15-15: LGTM!The boolean conversion is correct and handles the undefined case gracefully when
rolldownVersionis not available.
11-13: No action needed —rolldownVersionis correctly exported from Vite 8.0.0-beta.5.The import is valid and the conditional check at line 21 properly skips the vite-rsc test when using Rollup (not Rolldown).
commit: |
| const skipDev = new Set<string>(["auto-imports", "cached-handler"]); | ||
|
|
||
| const skipProd = new Set<string>(); | ||
| const skipProd = new Set<string>(isRolldown ? [] : ["vite-rsc"]); |
There was a problem hiding this comment.
@hi-ogawa i have excluded example (prod only) from matrix for now.
Locally, you can use
NITRO_VITE_PKG=vite7 pnpm vitest test/examples.test.ts -t vite-rsci cannot reproduce issue outside vitest...
| </li> | ||
| {/* <li> | ||
| Visit{" "} | ||
| <a href="?__rsc" target="_blank"> |
There was a problem hiding this comment.
Oh good catch. I fixed my template vitejs/vite-plugin-react#1041. Will fix it here in next PR on nitro.
|
Merging for faster iteration! LMK or feel free to make a followup PR to upgrade plugin to stable version. |
2 participants
🔗 Linked issue
@vitejs/plugin-rscnow assumes__VITE_ENVIRONMENT_RUNNER_IMPORT__global as convention forimport.meta.viteRsc.loadModuleimplementation (see vitejs/vite-plugin-react#1037). This PR defines the global inside nitro dev worker so RSC plugin works out of the box. (NOTE: This is not released yet, so using pkg.pr.new in the demo. After this approach is confirmed with Nitro, I can release new version)I noticed
servicesconfig is removed, so I added toexperimental.vite.services. RSC plugin still assumesrollupOptions.input.index, so adjustedcreateServiceEnvironmenttoo.I added
examples/vite-rscfor the demo.❓ Type of change
📚 Description
📝 Checklist