docs: RSC integration pitfalls from tutorial app#3087
Conversation
Document five real-world RSC integration issues discovered during react-webpack-rails-tutorial integration: - VM sandbox constraints: externals vs resolve.fallback, require() unavailability in server bundle - MessageChannel polyfill: BannerPlugin pattern for react-dom/server - 'use client' classification audit: .server.jsx naming collision, auto-classification gotchas, migration checklist - CI/test setup: Rails.env.local? guard, TCP readiness check, common CI failure table - Three-bundle architecture reference table comparing runtimes, constraints, and env vars across client/server/RSC bundles Closes #3076 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
WalkthroughAdded documentation clarifying Node renderer CI/test setup, VM-sandbox constraints for the server (SSR) bundle, a bundle-architecture reference distinguishing client/server/RSC runtimes, and a pre-migration audit for detecting client-only API usage before enabling React Server Components. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 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 |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Review: docs — RSC integration pitfallsOverall this is a valuable addition. The five pitfall categories are accurate, well-structured, and clearly sourced from real-world integration work. A few technical corrections are needed before merge: Issues (inline comments posted)
Minor observations (no inline comment)
|
Greptile SummaryThis PR adds documentation for five real-world RSC integration pitfalls discovered during a tutorial app migration: VM sandbox constraints ( The documentation is thorough and technically accurate overall, with a few minor clarifications worth considering before merge. Confidence Score: 5/5Safe to merge — all findings are P2 style/clarification suggestions with no correctness or data-integrity impact. All four comments are P2 (non-blocking suggestions): a missing exit-1 in example CI YAML, a note about env var scoping in the CI example, a misleading link text, and a minor framing issue around StaticRouter. None affect production behavior or introduce incorrect technical guidance. The core technical content (externals vs resolve.fallback, MessageChannel polyfill, Rails.env.local?, auto-classification) is accurate. docs/oss/building-features/node-renderer/basics.md — CI example could mislead users into incomplete RENDERER_PASSWORD setup across job steps.
|
| Filename | Overview |
|---|---|
| docs/oss/building-features/node-renderer/basics.md | Adds "CI and Test Environment Setup" section; CI readiness loop silently continues if renderer never starts, and RENDERER_PASSWORD is only scoped to the renderer step rather than the Rails test step. |
| docs/oss/migrating/rsc-troubleshooting.md | Adds externals vs resolve.fallback pitfall and MessageChannel BannerPlugin polyfill sections; technically accurate and well-explained. |
| docs/pro/react-server-components/rendering-flow.md | Adds bundle architecture reference table and externals pitfall callout; internal link text "VM Sandbox Constraints" doesn't match the target section title "Bundle Architecture Reference". |
| docs/pro/react-server-components/upgrading-existing-pro-app.md | Adds pre-migration client API audit section; StaticRouter is listed as a "client API requiring 'use client'" but it is a server-side router — the framing could mislead developers into thinking it always requires the directive. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Component file] --> B{Has 'use client'?}
B -- Yes --> C[Registered via ReactOnRails.register]
B -- No --> D[Registered via registerServerComponent]
C --> E[Client Component - Browser runtime]
D --> F[Server Component - RSC Bundle full Node.js]
D --> G[SSR via Server Bundle - VM sandbox no require]
subgraph ServerBundle[Server Bundle VM Sandbox]
G
H[resolve.fallback false - Node builtins to empty modules]
I[externals commonjs path - generates require crash]
J[BannerPlugin MessageChannel polyfill - injects before module init]
end
subgraph CI[CI Setup]
K[Start node-renderer.js background]
L[Poll port 3800 readiness]
M[Run Rails tests]
K --> L --> M
end
Reviews (1): Last reviewed commit: "docs: add RSC integration pitfalls from ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4d95bb7045
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/pro/react-server-components/upgrading-existing-pro-app.md (1)
33-78: LGTM! Pre-migration audit section is comprehensive and clear.The new section effectively addresses the
.server.jsxnaming collision and auto-classification gotcha. The client API checklist is thorough and the explanation clearly distinguishes between file suffix bundling (.server.jsxfor SSR) and RSC directive classification ('use client').Optional enhancement: Line 66 mentions runtime errors like "useState is not a function" -- consider adding a forward reference to the troubleshooting guide to help readers quickly resolve those errors:
📖 Optional cross-reference enhancement
There is no warning when a component is auto-classified as a server component. If it uses client APIs, it will fail at runtime with errors like "useState is not a function" or "Cannot read properties of undefined." + +> **Tip:** See [RSC Troubleshooting](../../oss/migrating/rsc-troubleshooting.md) for a complete error message catalog and solutions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/pro/react-server-components/upgrading-existing-pro-app.md` around lines 33 - 78, Add a brief forward reference to the troubleshooting guide where runtime errors are mentioned (the sentence containing "useState is not a function" / "Cannot read properties of undefined") so readers can quickly navigate to remediation steps; update the paragraph under "How auto-classification works" to append something like "See the troubleshooting guide for common RSC runtime errors" and link or point to the existing Troubleshooting section or doc page you maintain.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/pro/react-server-components/rendering-flow.md`:
- Around line 37-49: The doc incorrectly states that resolve.fallback is a
server-bundle setting; update the "Server Bundle (SSR)" and "Client Bundle"
descriptions and the "Key pitfall" paragraph to state that resolve.fallback
(e.g., resolve.fallback: { fs: false, path: false, stream: false }) is used by
the client bundle to avoid generating require() calls for browser builds, while
the server bundle uses target: 'node' (so Node builtins are natively supported
and do not need resolve.fallback) and that using externals on the server can
produce require() calls that fail in non-Node VM contexts; adjust the table row
for "Node builtins" and the pitfall text to clearly distinguish "what webpack
generates (externals)" vs "how webpack handles imports (resolve.fallback)" for
each bundle, referencing the "Client Bundle", "Server Bundle (SSR)",
`resolve.fallback`, `externals`, and `target: 'node'` tokens to locate the
edits.
---
Nitpick comments:
In `@docs/pro/react-server-components/upgrading-existing-pro-app.md`:
- Around line 33-78: Add a brief forward reference to the troubleshooting guide
where runtime errors are mentioned (the sentence containing "useState is not a
function" / "Cannot read properties of undefined") so readers can quickly
navigate to remediation steps; update the paragraph under "How
auto-classification works" to append something like "See the troubleshooting
guide for common RSC runtime errors" and link or point to the existing
Troubleshooting section or doc page you maintain.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bdf09b4d-2356-4b90-9656-174348deaf9f
📒 Files selected for processing (4)
docs/oss/building-features/node-renderer/basics.mddocs/oss/migrating/rsc-troubleshooting.mddocs/pro/react-server-components/rendering-flow.mddocs/pro/react-server-components/upgrading-existing-pro-app.md
- Separate StaticRouter from client API list (it's an SSR router, not a client API) - Add failure exit to CI readiness loop when renderer never starts - Fix inaccurate additionalContext timing claim (both approaches work) - Fix resolve.fallback description (omits module, not empty object) - Fix link text mismatch (VM Sandbox Constraints → Bundle Architecture Reference) - Add note about RENDERER_PASSWORD needing job-level scope in CI - Scope externals guidance: note that externals work with supportModules enabled - Clarify resolve.fallback usage across client and server bundles Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Review feedback addressed in 4281a61All 11 review threads resolved. Changes in a single commit: Technical corrections (4):
Clarifications (4): 3 duplicate threads (greptile, codex) replied to and resolved. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/oss/migrating/rsc-troubleshooting.md (1)
736-736: Align the fallback snippet with the detailed section.Line 736 omits
stream: false, but the canonical config later includes it. Please make both examples identical to avoid partial copy/paste fixes.Suggested docs diff
-| `ReferenceError: require is not defined` | Server bundle webpack config uses `externals` for Node builtins, generating `require()` calls that fail in VM sandbox | Use `resolve.fallback: { path: false, fs: false }` instead of `externals`. See [Handling Node Builtins](`#handling-node-builtins----externals-vs-resolvefallback`) | +| `ReferenceError: require is not defined` | Server bundle webpack config uses `externals` for Node builtins, generating `require()` calls that fail in VM sandbox | Use `resolve.fallback: { path: false, fs: false, stream: false }` instead of `externals`. See [Handling Node Builtins](`#handling-node-builtins----externals-vs-resolvefallback`) |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/oss/migrating/rsc-troubleshooting.md` at line 736, The docs table entry shows using resolve.fallback: { path: false, fs: false } but omits stream, causing inconsistency with the canonical example; update the short table snippet to include stream: false so it exactly matches the detailed section (use the same resolve.fallback keys: path, fs, and stream) and ensure the phrase references resolve.fallback consistently to avoid partial copy/paste errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/oss/migrating/rsc-troubleshooting.md`:
- Line 736: The docs table entry shows using resolve.fallback: { path: false,
fs: false } but omits stream, causing inconsistency with the canonical example;
update the short table snippet to include stream: false so it exactly matches
the detailed section (use the same resolve.fallback keys: path, fs, and stream)
and ensure the phrase references resolve.fallback consistently to avoid partial
copy/paste errors.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dc8d1cc6-06e4-4922-8172-57fd747a463d
📒 Files selected for processing (4)
docs/oss/building-features/node-renderer/basics.mddocs/oss/migrating/rsc-troubleshooting.mddocs/pro/react-server-components/rendering-flow.mddocs/pro/react-server-components/upgrading-existing-pro-app.md
✅ Files skipped from review due to trivial changes (1)
- docs/oss/building-features/node-renderer/basics.md
Review: RSC Integration Pitfalls DocumentationOverall this is a high-quality, clearly structured documentation PR. The content is technically accurate and directly addresses real integration pain points. A few issues worth addressing before merge: Functional issue (blocks correct CI setup)
Minor robustness issues
Documentation completeness
Nits (no action required)
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4281a61454
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Fix .server.jsx rename recommendation: renaming to .ssr.jsx breaks
auto-bundling (packs_generator.rb uses Dir.glob("*.server.*"))
- Add missing React 18/19 hooks to client API audit list:
useTransition, useDeferredValue, useId, useOptimistic, useFormStatus
- Add router hooks to grep checklist: useNavigate, useLocation, useParams
- Restructure CI YAML to show RENDERER_PASSWORD at job level
- Add --max-time 2 to curl readiness probe to prevent stalled hangs
- Add MessageChannel polyfill sync semantics caveat with
additionalContext fallback note
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Review feedback addressed in 08bd481All 6 remaining review threads resolved. Changes in a single commit: Critical fix (1):
Completeness fixes (2): CI snippet fixes (2): Documentation caveat (1): |
Review: docs/RSC integration pitfallsThis is a high-quality, battle-tested set of documentation additions. The three-bundle architecture reference table is particularly valuable — the server bundle / RSC bundle confusion is one of the hardest pitfalls to debug, and having a single-page diff laid out in a table will save a lot of time. The Four specific issues flagged via inline comments:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 08bd4814d2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…on order - Split useFormStatus into separate "React DOM hooks" bullet with correct import path (react-dom, not react) to prevent wrong imports - Lead with additionalContext + native MessageChannel as recommended approach; demote BannerPlugin synchronous polyfill to fallback option Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Review feedback addressed in 25c3781Third round — 4 of 6 threads resolved. Changes in a single commit: Fixes (2):
Declined with rationale (2): Remaining (2 unresolved, need decision): |
| ```yaml | ||
| # .github/workflows/test.yml (GitHub Actions example) | ||
| jobs: | ||
| test: |
There was a problem hiding this comment.
The YAML snippet is missing the required runs-on: key for the job. GitHub Actions will reject this workflow without it, making this example non-functional if copy-pasted directly.
| test: | |
| test: | |
| runs-on: ubuntu-latest | |
| env: |
(Users may need to substitute their actual runner label, but the key itself is required.)
|
|
||
| ```js | ||
| // In serverWebpackConfig.js -- tells webpack to provide empty modules | ||
| resolve: { |
There was a problem hiding this comment.
The inline code comment contradicts the explanatory paragraph below it. resolve.fallback: false does not tell webpack to provide empty modules — it tells webpack to omit the module entirely (no shim is generated, and any direct require('path') that can't be tree-shaken away becomes a build warning/error). An empty module shim would be resolve.fallback: { path: {} } or a stub file.
The paragraph at line 876 correctly says "no fallback shim is bundled", so the comment and the explanation are contradicting each other.
| resolve: { | |
| // In serverWebpackConfig.js -- tells webpack to omit these modules (no require() call emitted) |
| **Root cause:** `react-dom/server.browser.js` (used for SSR streaming) instantiates `MessageChannel` at **module load time** for React's internal scheduler. The VM sandbox does not provide `MessageChannel` as a global, and `supportModules` does not include it. | ||
|
|
||
| **Fix:** Use webpack's `BannerPlugin` to inject a minimal `MessageChannel` polyfill at the top of the server bundle. The polyfill only needs to support `port2.postMessage` triggering `port1.onmessage`, which is all React's scheduler requires: | ||
|
|
There was a problem hiding this comment.
The section structure is confusing: the Fix: label leads users into the BannerPlugin (synchronous polyfill) approach, and the Recommended approach: using additionalContext only appears 35 lines later after the full code example. Most users will implement the first **Fix:** they see and never read to the recommended option.
Suggest flipping the order:
- Lead with
additionalContext+ nativeMessageChannelas the primary fix - Follow with the BannerPlugin as the fallback for environments where renderer config can't be changed
This matches the intent of the > When to use the BannerPlugin polyfill: callout at the bottom, which already correctly frames BannerPlugin as the exception rather than the rule.
| - **Router client APIs**: `useNavigate`, `useLocation`, `useParams` | ||
| - **SSR entry-point files** using `StaticRouter`: these are SSR wrappers, not RSC server components — see the `.server.jsx` naming collision below | ||
| - **Event handlers**: `onClick`, `onChange`, `onSubmit`, etc. | ||
| - **Browser APIs**: `window`, `document`, `localStorage`, `fetch` in effects |
There was a problem hiding this comment.
fetch is not a browser-only API — it has been a Node.js global since v18.0.0 (built-in, unflagged since Node 21). It works in Server Components and RSC bundles. Listing it under "Browser APIs" is misleading and may cause developers to add unnecessary 'use client' directives to components that call fetch directly in server context (which is a common and encouraged RSC pattern).
The "in effects" qualifier partially saves this, but useEffect is already listed separately and would catch that pattern. I'd suggest dropping fetch from this list, or scoping it more precisely to fetch called inside useEffect (i.e., the client effect pattern), not fetch in general.
|
|
||
| Before proceeding to Step 1: | ||
|
|
||
| - [ ] Search your component source files for `useState`, `useEffect`, `useContext`, `useSelector`, `useDispatch`, `useTransition`, `useDeferredValue`, `useNavigate`, `useLocation`, `useParams`, `ReactOnRails.getStore` |
There was a problem hiding this comment.
The grep checklist is missing several hooks that appear in the "What to look for" list just above it. Users following only the checklist will miss:
useRef,useReducer,useCallback,useMemo(common hooks)useId,useOptimistic(React 18/19)useFormStatus(react-dom)ReactOnRails.authenticityToken
Suggest expanding the search pattern:
| - [ ] Search your component source files for `useState`, `useEffect`, `useContext`, `useSelector`, `useDispatch`, `useTransition`, `useDeferredValue`, `useNavigate`, `useLocation`, `useParams`, `ReactOnRails.getStore` | |
| - [ ] Search your component source files for `useState`, `useEffect`, `useContext`, `useRef`, `useReducer`, `useCallback`, `useMemo`, `useTransition`, `useDeferredValue`, `useId`, `useOptimistic`, `useFormStatus`, `useSelector`, `useDispatch`, `useNavigate`, `useLocation`, `useParams`, `ReactOnRails.getStore`, `ReactOnRails.authenticityToken` |
Review: docs: RSC integration pitfalls from tutorial appOverall this is high-quality documentation that fills real gaps — the three-bundle architecture table, the Issues foundBlocking / correctness:
Non-blocking / improvements:
What looks good
|
- Use curl --http2-prior-knowledge for readiness probe since the renderer uses cleartext HTTP/2 (h2c) and rejects HTTP/1.1 requests - Remove incorrect RENDERER_BUNDLE_PATH env var (deprecated alias for serverBundleCachePath cache dir, not webpack output path) - Replace with guidance to use rake pre_stage_bundle_for_node_renderer to symlink webpack output into the renderer cache before starting Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Review feedback addressed in bf26cb1Fourth round — all 6 threads now resolved. Fixes (2):
Follow-up issue filed:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bf26cb1820
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| - Generated using the RSC Webpack Loader which transforms client components into references | ||
| - Used specifically for generating RSC payloads | ||
| - Configured with `react-server` condition to enable RSC-specific code paths that tell the runtime that this bundle is used for RSC payload generation. | ||
| - **Runtime: Full Node.js** -- Node builtins (`path`, `fs`, `stream`) and `require()` work normally |
There was a problem hiding this comment.
This line says the RSC bundle runs in full Node.js with normal require(), but the renderer executes both server and RSC bundles inside VM contexts: Rails sends both bundle hashes as dependencyBundleTimestamps (react_on_rails_pro/lib/react_on_rails_pro/utils.rb:168-177), handleRenderRequest builds VMs for all bundle paths (packages/react-on-rails-pro-node-renderer/src/worker/handleRenderRequest.ts:218-271), and execution happens via vm.createContext/vm.runInContext (packages/react-on-rails-pro-node-renderer/src/worker/vm.ts:240,148). This guidance can cause users to configure the RSC bundle as if Node builtins and require() are always available, leading to runtime failures unless they explicitly inject the needed globals/modules.
Useful? React with 👍 / 👎.
|
|
||
| ```js | ||
| // In serverWebpackConfig.js -- tells webpack to provide empty modules | ||
| resolve: { |
There was a problem hiding this comment.
The inline comment "tells webpack to provide empty modules" contradicts the false values below. In webpack 5, resolve.fallback: { path: false } means no fallback is provided — not that an empty module is injected. If code directly imports path, webpack will throw a "Module not found" build error. The correct phrasing is something like "tells webpack not to polyfill these Node builtins":
| resolve: { | |
| // In serverWebpackConfig.js -- tells webpack not to polyfill these Node builtins |
The subsequent prose explanation on line 876 ("omit the module at build time") has the same nuance issue — it's technically "no polyfill is provided" rather than "the module is omitted." The important and correct point is that no require() call is emitted in the output bundle, which should be kept.
| This injects the polyfill as raw JavaScript at the top of the bundle output, ensuring `MessageChannel` is defined before any module code executes. | ||
|
|
||
| **Recommended approach:** Use `additionalContext` in the renderer config to inject Node.js' native `MessageChannel` into the VM sandbox. This provides correct async scheduling (macrotask delivery), which React's scheduler depends on. The `BannerPlugin` polyfill above delivers messages synchronously — this works for current SSR streaming scenarios but may cause subtle rendering bugs if React yields mid-render and re-enters the scheduler. | ||
|
|
There was a problem hiding this comment.
The recommended approach (additionalContext) is buried after the BannerPlugin walkthrough, which means many readers will stop reading after the fix and use the synchronous polyfill without seeing the warning. Consider inverting the order — lead with additionalContext as the preferred path, then describe BannerPlugin as the escape hatch for when renderer config is unavailable. The current structure nudges users toward the approach that can cause "subtle rendering bugs."
Also worth calling out explicitly: the synchronous postMessage means React's scheduler fires microtask-style (in the same call stack) instead of via a macrotask. This can mask real scheduling issues and cause stack overflows on deeply recursive RSC trees — not just a theoretical concern.
| steps: | ||
| - name: Start Node Renderer | ||
| run: | | ||
| node client/node-renderer.js & |
There was a problem hiding this comment.
If node-renderer.js fails immediately (missing file, bad config, syntax error), the background process exits but the shell continues, and the readiness loop burns all 30 seconds before failing. Consider adding a brief initial wait and an explicit exit-status check:
| node client/node-renderer.js & | |
| node client/node-renderer.js & | |
| RENDERER_PID=$! | |
| sleep 1 | |
| # Bail early if the process already crashed | |
| if ! kill -0 "$RENDERER_PID" 2>/dev/null; then | |
| echo "Node Renderer process exited immediately — check logs above" >&2 | |
| exit 1 | |
| fi |
This surfaces startup crashes in ~1 second instead of 30.
|
|
||
| - **React hooks** (`import { ... } from 'react'`): `useState`, `useEffect`, `useContext`, `useRef`, `useReducer`, `useCallback`, `useMemo`, `useTransition`, `useDeferredValue`, `useId`, `useOptimistic` | ||
| - **React DOM hooks** (`import { ... } from 'react-dom'`): `useFormStatus` | ||
| - **React on Rails client APIs**: `ReactOnRails.getStore()`, `ReactOnRails.authenticityToken()` |
There was a problem hiding this comment.
In React 19, useFormState was renamed useActionState and moved from react-dom to react. useFormStatus stays in react-dom. Worth mentioning both since codebases targeting React 18 may use useFormState and the rename is a common migration trip-wire:
| - **React on Rails client APIs**: `ReactOnRails.getStore()`, `ReactOnRails.authenticityToken()` | |
| - **React DOM hooks** (`import { ... } from 'react-dom'`): `useFormStatus` | |
| - **React 19 form hooks** (`import { ... } from 'react'`): `useActionState` (was `useFormState` from `react-dom` in React 18) |
Review: docs: RSC integration pitfalls from tutorial appGood set of real-world pitfalls — the bundle architecture reference table and the Technical accuracy
MessageChannel polyfill ordering (inline comment on
ResilienceCI script: no fast-fail on immediate renderer crash (inline comment on Minor nits (no inline comment needed)
|
…ages * origin/main: Fix initial page startup race for late-loading client bundles (#3151) chore: apply prettier formatting to tracked docs files (#3153) docs: comprehensive RSC API documentation and registration consolidation (#3140) Split rspec-package-tests into parallel generator/unit shards (#3134) fix: add concurrency groups to long-running CI workflows (#3133) refactor: add RenderRequest, JsCodeBuilder, and RenderingStrategy abstractions (#3094) fix: address deferred review items from PR #2849 (#3093) Add complimentary OSS license policy for React on Rails Pro (#3123) fix: centralize CI docs-only detection and add CLI flag validation (#3091) refactor: replace stub-throw + Object.assign with capability-based composition (#3096) Enhance address-review with parallel fixes, self-review, and Greptile verification (#3121) fix: Doctor no longer fails custom projects for missing bin/dev (#3117) fix: cap webpack <5.106.0 to prevent ExecJS SSR breakage (#3095) Add Rspack + RSC compatibility tests and documentation (#1828) (#3120) Add error scenarios hub and test pages (#2497) docs: document polyfill requirements for web-targeted server bundles (#3092) docs: RSC integration pitfalls from tutorial app (#3087) docs: fix render function/helper API documentation (#3088) Doctor: accept TS/TSX server bundle suffixes (#3111) feat: add CI guard requiring sidebar updates when adding docs (#3089)
Applies the documentation review comments surfaced against PR #3087: - Add missing `runs-on: ubuntu-latest` to the CI workflow example in node-renderer/basics.md so the snippet is copy-paste valid. - Correct the `resolve.fallback: false` inline comment in rsc-troubleshooting.md to match the surrounding explanation ("omit the module" rather than "provide empty modules"). - Expand the upgrading-existing-pro-app audit checklist to include the React, React DOM, router, and ReactOnRails hook names that were listed in "What to look for" but missing from the checklist. - Reorder the MessageChannel troubleshooting section to lead with the recommended `additionalContext` fix and demote BannerPlugin to a fallback, so users encounter the preferred option first. - Clarify `fetch` under "Browser APIs": it is a Node.js global since v18 and works in Server Components; only flag it when called inside a `useEffect` (already covered by the hooks list). Fixes #3155 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Applies the documentation review comments surfaced against PR #3087: - Add missing `runs-on: ubuntu-latest` to the CI workflow example in node-renderer/basics.md so the snippet is copy-paste valid. - Correct the `resolve.fallback: false` inline comment in rsc-troubleshooting.md to match the surrounding explanation ("omit the module" rather than "provide empty modules"). - Expand the upgrading-existing-pro-app audit checklist to include the React, React DOM, router, and ReactOnRails hook names that were listed in "What to look for" but missing from the checklist. - Reorder the MessageChannel troubleshooting section to lead with the recommended `additionalContext` fix and demote BannerPlugin to a fallback, so users encounter the preferred option first. - Clarify `fetch` under "Browser APIs": it is a Node.js global since v18 and works in Server Components; only flag it when called inside a `useEffect` (already covered by the hooks list). Fixes #3155 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
## Summary Applies the five documentation review comments from PR #3087 that were triaged to issue #3155: - `docs/oss/building-features/node-renderer/basics.md` — Add missing `runs-on: ubuntu-latest` to the GitHub Actions workflow example so the snippet is copy-paste valid. - `docs/oss/migrating/rsc-troubleshooting.md` — Correct the `resolve.fallback: false` inline comment to say "omit these modules (no require() call emitted)", matching the paragraph below it (no empty-module shim is generated). - `docs/pro/react-server-components/upgrading-existing-pro-app.md` — Expand the audit checklist to include the React hooks (`useRef`, `useReducer`, `useCallback`, `useMemo`, `useId`, `useOptimistic`), React DOM hooks (`useFormStatus`), and `ReactOnRails.authenticityToken` that were listed under "What to look for" but missing from the checklist. - `docs/oss/migrating/rsc-troubleshooting.md` — Reorder the MessageChannel Not Defined section to lead with the recommended `additionalContext` fix (injecting Node's native `MessageChannel` from `node:worker_threads`) and demote BannerPlugin to a labeled fallback, so readers encounter the preferred approach first. - `docs/pro/react-server-components/upgrading-existing-pro-app.md` — Clarify `fetch` under "Browser APIs": it's a Node.js global since v18 and works in Server Components. The list now only flags `fetch` calls inside `useEffect`, which are already captured by the hooks list above. Fixes #3155 ## Test plan - [x] Prettier check (`npx prettier --check`) passes on all three modified docs - [x] Lefthook hooks (trailing-newlines, markdown-links, prettier) passed on commit - [x] No code changes — documentation only 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Documentation-only updates that adjust examples and recommended fixes; no runtime behavior changes, so risk is low aside from potential reader confusion if guidance is misapplied. > > **Overview** > Updates RSC/Node Renderer docs to make setup and troubleshooting guidance more accurate and copy/paste-ready. > > The CI workflow snippet now includes `runs-on: ubuntu-latest`, the webpack `resolve.fallback: false` explanation is clarified, and the `MessageChannel is not defined` section is reordered to recommend injecting Node’s native `MessageChannel` via `additionalContext` (with `BannerPlugin` polyfill as an explicit fallback). The Pro RSC upgrade guide expands the client-API audit checklist to cover additional hooks/APIs and clarifies that `fetch` is usable in Server Components (flag it only when used in `useEffect`). > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 6ba6295. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Specified explicit runner environment for Node Renderer test workflow * Updated MessageChannel troubleshooting to recommend injecting Node’s MessageChannel at the renderer level, with the webpack polyfill noted as a fallback * Clarified that fetch is usable in Server Components and expanded the audit checklist to include additional React hooks and related APIs <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 participant
Summary
Documents five real-world RSC integration pitfalls discovered during the react-webpack-rails-tutorial RSC integration:
rendering-flow.md,rsc-troubleshooting.md): Documents whyexternalsmust not be used for Node builtins in the server bundle (generatesrequire()calls that fail in the VM sandbox), and thatresolve.fallback: falseis the correct approachrsc-troubleshooting.md): Documents theBannerPluginpolyfill pattern forreact-dom/server.browser.jswhich needsMessageChannelat module load time'use client'classification audit (upgrading-existing-pro-app.md): Adds a pre-migration section covering the.server.jsxnaming collision, how auto-classification works, and a checklist of client APIs to audit forbasics.md): DocumentsRails.env.local?vsRails.env.development?, TCP readiness checks,RENDERER_PASSWORDin CI, and a common CI failures tablerendering-flow.md): Adds a reference table comparing all three bundles across runtime, Node builtin handling,require()availability, CSS extraction, and env varsCloses #3076
Test plan
🤖 Generated with Claude Code
Note
Low Risk
Low risk because changes are documentation-only, though reviewers should sanity-check technical accuracy of the new webpack/VM and CI guidance.
Overview
Adds a new CI/test setup section for the Node Renderer, including
Rails.env.local?initializer guarding, a GitHub Actions example that starts the renderer with an HTTP/2 (h2c) readiness probe,RENDERER_PASSWORDguidance, and a common-failures table.Expands RSC troubleshooting and Pro RSC docs to clarify bundle runtime differences (client vs SSR server bundle VM sandbox vs full-Node RSC bundle), document
require()/missing-global failure modes, recommendresolve.fallback: falseover webpackexternalsfor Node builtins in the server bundle, and add guidance for handling missingMessageChannel(polyfill viaBannerPlugin/ preferadditionalContext).Adds a pre-migration checklist for existing Pro apps to audit for client-only APIs and warns about the
.server.jsxnaming collision that can cause unintended Server Component classification without'use client'.Reviewed by Cursor Bugbot for commit bf26cb1. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit