Add React Server Components with React on Rails Pro#722
Add React Server Components with React on Rails Pro#722
Conversation
Upgrade from react_on_rails to react_on_rails_pro gem (16.5.1) and
corresponding npm packages. Add RSC infrastructure with a demo page
at /server-components that showcases:
- Server components using Node.js os module and lodash (never shipped
to client)
- Async data fetching with Suspense streaming (comments from Rails API)
- Interactive client components ('use client' TogglePanel) mixed with
server-rendered content (donut pattern)
- Markdown rendering with marked + sanitize-html on server only
Key changes:
- Three-bundle build: client, server, and RSC bundles via Rspack
- Custom RspackRscPlugin for manifest generation (the standard
RSCWebpackPlugin uses webpack internals incompatible with Rspack)
- 'use client' directives on all existing client component entry points
- Alias react-on-rails to react-on-rails-pro in webpack resolve to
handle third-party packages (rescript-react-on-rails)
- Dedicated rsc-bundle.js entry with registerServerComponent
- RSC payload route and client-side registration
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
🚀 Quick Review App CommandsWelcome! Here are the commands you can use in this PR:
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds React Server Components (RSC) support: swaps to react_on_rails_pro, adds RSC webpack configs and an Rspack plugin, introduces a node renderer and RSC packs/entrypoints, adds a server-components demo (server + client pieces), and wires initializer, routes, and dev processes. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant Rails
participant NodeRSC as Node RSC Renderer
participant ClientLoader as Client Loader
Browser->>Rails: GET /server-components
activate Rails
Rails->>NodeRSC: Request RSC render for ServerComponentsPage
deactivate Rails
activate NodeRSC
NodeRSC->>NodeRSC: Render ServerInfo (sync)
NodeRSC->>NodeRSC: Await CommentsFeed (fetch, markdown/sanitize)
NodeRSC->>Browser: Stream RSC/Flight payload
deactivate NodeRSC
Browser->>ClientLoader: Fetch client manifest & chunks
ClientLoader->>Browser: Load TogglePanel chunk
Browser->>Browser: Hydrate TogglePanel (interactive)
sequenceDiagram
participant Bundler
participant RscConfig
participant Plugin as RspackRscPlugin
participant Output
Bundler->>RscConfig: Load rscWebpackConfig (RSC_BUNDLE_ONLY)
RscConfig->>RscConfig: Strip CSS, set react-server condition, enforce single chunk
Bundler->>Plugin: Run RspackRscPlugin during compilation
Plugin->>Plugin: Scan chunks/modules for 'use client'
Plugin->>Output: Emit react-client-manifest.json (and SSR manifest when applicable)
Output->>Bundler: RSC manifests & bundle produced
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
PR Review: Add React Server Components with React on Rails ProThis is a well-structured PR that introduces RSC support cleanly. The three-bundle pipeline design and the custom Critical
Bugs
Debugging artifacts left in production code
Minor / Cleanup
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
client/app/bundles/server-components/components/CommentsFeed.jsx
Outdated
Show resolved
Hide resolved
client/app/bundles/server-components/components/CommentsFeed.jsx
Outdated
Show resolved
Hide resolved
client/app/bundles/server-components/components/CommentsFeed.jsx
Outdated
Show resolved
Hide resolved
client/app/bundles/comments/startup/RouterApp/ror_components/RouterApp.server.jsx
Show resolved
Hide resolved
![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: 4d09e13058
ℹ️ 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".
client/app/bundles/server-components/components/CommentsFeed.jsx
Outdated
Show resolved
Hide resolved
Greptile SummaryThis PR introduces React Server Components (RSC) support via
Confidence Score: 3/5Not safe to merge until the hardcoded localhost URL and missing error handling in CommentsFeed are fixed Two P1 defects exist in CommentsFeed.jsx: a hardcoded localhost URL that breaks all non-local deployments and a missing response.ok guard that causes silent crashes on API errors. These are on the primary path of the new RSC demo feature. All other changes (webpack configs, plugin, Rails routes, component structure) are well-structured and low risk. client/app/bundles/server-components/components/CommentsFeed.jsx requires fixes to the fetch call before merge Important Files Changed
Sequence DiagramsequenceDiagram
participant Browser
participant Rails
participant RSCRenderer as RSC Node Renderer
participant API as Rails API
Browser->>Rails: GET /server-components
Rails-->>Browser: HTML shell + JS bundles (client + rsc-client-components)
Browser->>Rails: GET /rsc_payload/ServerComponentsPage
Rails->>RSCRenderer: Render ServerComponentsPage (rsc-bundle.js)
RSCRenderer->>API: fetch /comments.json (server-side, hardcoded localhost:3000)
API-->>RSCRenderer: JSON comments array
RSCRenderer-->>Rails: React Flight payload (streamed chunks)
Rails-->>Browser: Streamed RSC payload
Browser->>Browser: Hydrate TogglePanel ('use client')
Note over Browser: ServerInfo and CommentsFeed<br/>ship zero client JS
Reviews (1): Last reviewed commit: "Add React Server Components with React o..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
client/app/bundles/server-components/components/CommentsFeed.jsx
Outdated
Show resolved
Hide resolved
client/app/bundles/server-components/components/CommentsFeed.jsx
Outdated
Show resolved
Hide resolved
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
client/app/bundles/server-components/components/CommentsFeed.jsx
Outdated
Show resolved
Hide resolved
- Move eslint-disable after 'use client' directive in SimpleCommentScreen - Add no-promise-executor-return disable for setTimeout in CommentsFeed - Replace array index key with semantic key in ServerInfo - Add PropTypes to TogglePanel component - Fix import ordering in stimulus-bundle.js Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
client/app/bundles/server-components/components/CommentsFeed.jsx
Outdated
Show resolved
Hide resolved
client/app/bundles/server-components/components/CommentsFeed.jsx
Outdated
Show resolved
Hide resolved
client/app/bundles/comments/startup/RouterApp/ror_components/RouterApp.server.jsx
Show resolved
Hide resolved
- Use single-line comment eslint-disable before 'use client' directive (file-level rules must be disabled before line 1) - Suppress react/no-danger for sanitized HTML in CommentsFeed Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Code ReviewGreat direction — RSC support on top of React on Rails Pro is a meaningful addition and the three-bundle architecture is well-structured. A few issues need attention before merging: Bugs / Must-Fix1. Hardcoded 2. 3. Code Quality4. 5. Artificial 800 ms delay in Security / Minor6. Unrestricted 7. SSR manifest always emitted as Observations (no action needed)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (8)
config/webpack/commonWebpackConfig.js (1)
11-17: Consider using a broader alias pattern for complete coverage.The current aliases handle exact imports and the
/node_packagesubpath, but third-party packages might import other subpaths (e.g.,react-on-rails/someOtherPath). A regex-based alias could provide more comprehensive coverage.♻️ Alternative: broader alias pattern
If you encounter issues with other subpath imports, consider:
alias: { 'react-on-rails$': 'react-on-rails-pro', - 'react-on-rails/node_package': 'react-on-rails-pro/node_package', + 'react-on-rails/': 'react-on-rails-pro/', },Note: The trailing slash pattern behavior varies between webpack/rspack versions. Test this if you encounter additional subpath resolution issues.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/webpack/commonWebpackConfig.js` around lines 11 - 17, The current webpack alias object only maps exact imports ('react-on-rails$' and 'react-on-rails/node_package') and misses other subpath imports; update the alias entries in the alias object to use a regex/broader pattern so any import of react-on-rails and its subpaths (e.g., react-on-rails/... ) is redirected to react-on-rails-pro and its corresponding subpath, replacing or supplementing the existing keys in the alias object to ensure complete coverage across imports; test subpath resolution behavior in your webpack/rspack version after changing the alias.config/webpack/rspackRscPlugin.js (1)
20-24: Synchronous file I/O may impact build performance at scale.Using
fs.openSync/readSync/closeSyncblocks the event loop. While caching mitigates repeated reads, the initial scan of many files could slow builds. This is acceptable for most projects but worth noting for large codebases.For future optimization, consider using
fs.promiseswithPromise.allif build times become a concern, though this would require restructuring theprocessAssetshook to be async.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/webpack/rspackRscPlugin.js` around lines 20 - 24, The current synchronous file reads (fs.openSync, fs.readSync, fs.closeSync) in the RSC scanning logic block that reads the first ~200 bytes of filePath block the event loop and can degrade large builds; change this to use fs.promises (e.g., fs.promises.open andfilehandle.read or fs.promises.readFile with a slice) and perform reads concurrently with Promise.all so many files are scanned in parallel, and update the surrounding processAssets hook (or the function that calls this scan) to be async/await-compatible so it can await the promise-based reads without blocking; keep the existing cache semantics and only switch the low-level I/O to the promise-based APIs while preserving the logic that checks for 'use client' at the top of filePath.package.json (1)
132-132: Upgradeeslint-plugin-react-hooksto the latest version for React 19 compatibility.The current version
^4.6.0does not fully support React 19's new hooks likeuseActionState,useFormStatus, anduseOptimistic. The latest stable version (7.0.1) provides complete support for React 19, including validation rules for React Compiler features introduced in React 19.📦 Suggested upgrade
- "eslint-plugin-react-hooks": "^4.6.0", + "eslint-plugin-react-hooks": "^7.0.0",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` at line 132, Update the eslint-plugin-react-hooks dependency in package.json to the latest compatible release (replace the "eslint-plugin-react-hooks": "^4.6.0" entry) so the project uses the React 19–compatible validator (e.g., 7.0.1); modify the package.json dependency line for eslint-plugin-react-hooks and then run your package manager (npm/yarn/pnpm) to install and update lockfiles, and run the linter to confirm no new rule errors from the updated plugin.app/controllers/pages_controller.rb (1)
41-41: Skipset_commentsfor the newserver_componentsaction.Line 41 introduces an action with no use of
@comments, but it still incursbefore_action :set_comments. Consider excluding this action to avoid unnecessary DB work.♻️ Proposed change
- before_action :set_comments + before_action :set_comments, except: %i[server_components]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/controllers/pages_controller.rb` at line 41, The new controller action server_components is being run through the before_action :set_comments even though it does not use `@comments`; update the before_action declaration to skip set_comments for this action by adding an except/only exemption (e.g., modify the before_action :set_comments line to exclude :server_components) so the database work is avoided when server_components is invoked.app/views/pages/server_components.html.erb (1)
2-4: Gate tracing by environment.Line 4 uses
trace: trueunconditionally. Prefer enabling trace only in development to reduce noise and overhead.♻️ Proposed change
<%= react_component("ServerComponentsPage", prerender: false, - trace: true, + trace: Rails.env.development?, id: "ServerComponentsPage-react-component-0") %>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/views/pages/server_components.html.erb` around lines 2 - 4, The react_component call currently hardcodes trace: true; change it to enable tracing only in development by replacing the literal with a conditional using the Rails environment (e.g., set trace to Rails.env.development? or equivalent), updating the react_component("ServerComponentsPage", prerender: false, trace: ...) invocation so trace is true only when Rails.env.development? and false otherwise.client/app/bundles/server-components/components/TogglePanel.jsx (1)
10-29: Expose expanded/collapsed state to assistive tech.Please add
aria-expanded(and ideallyaria-controls) on the toggle button so screen readers get the panel state.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/app/bundles/server-components/components/TogglePanel.jsx` around lines 10 - 29, The TogglePanel component's toggle button needs ARIA attributes so assistive tech can read its state: update the button (where setIsOpen is used and isOpen is referenced) to include aria-expanded={isOpen} and aria-controls pointing to the panel's id, and give the panel div (the conditional container that renders {children}) a matching id; implement the id either by accepting a prop (e.g., panelId) or generating a stable id inside the component (e.g., React's useId) and use that id on the div so aria-controls links to it.config/webpack/rscWebpackConfig.js (2)
89-90: Don't replace the inheritedoptimizationobject.This drops whatever
commonWebpackConfig()already set up. Onlyminimizeneeds to change here, so merge into the existing object instead of resetting it wholesale.Suggested diff
- rscConfig.optimization = { minimize: false }; + rscConfig.optimization = { + ...(rscConfig.optimization || {}), + minimize: false, + };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/webpack/rscWebpackConfig.js` around lines 89 - 90, The code currently overwrites the inherited optimization object (rscConfig.optimization = { minimize: false }), dropping settings established by commonWebpackConfig(); instead merge into the existing object by preserving rscConfig.optimization and only changing minimize (e.g., assign rscConfig.optimization = { ...rscConfig.optimization, minimize: false } or set rscConfig.optimization.minimize = false) so other optimization defaults remain intact; update the rscWebpackConfig code that sets rscConfig.optimization accordingly.
29-33: Mirror the server-bundle path diagnostics here.
config/webpack/serverWebpackConfig.js:52-68derives the expected bundle path fromconfig.source_pathandconfig.source_entry_path. Hard-codingclient/app/packs/rsc-bundle.jsmakes this failure misleading for any non-default Shakapacker layout.Suggested diff
const rscEntry = rscConfig.entry['rsc-bundle']; if (!rscEntry) { + const sourcePath = config.source_path || 'client/app'; + const entryPath = config.source_entry_path || 'packs'; + const fullPath = `${sourcePath}/${entryPath}/rsc-bundle.js`; throw new Error( - 'RSC bundle entry not found. Ensure client/app/packs/rsc-bundle.js exists.', + `RSC bundle entry 'rsc-bundle' not found.\n` + + `Expected file: ${fullPath}\n` + + `Current source_path: ${config.source_path}\n` + + `Current source_entry_path: ${config.source_entry_path}\n` + + `Verify:\n` + + `1. The rsc-bundle.js file exists at the expected location\n` + + `2. nested_entries is configured correctly in shakapacker.yml\n` + + `3. The file is properly exported from your entry point`, ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/webpack/rscWebpackConfig.js` around lines 29 - 33, The error thrown when rscEntry is missing should mirror the server-bundle diagnostics by deriving the expected RSC bundle path from the same config values instead of hard-coding "client/app/packs/rsc-bundle.js"; update the check around rscEntry (rscEntry and rscConfig.entry) to compute the expected path using config.source_path and config.source_entry_path (same approach used in serverWebpackConfig's bundle path logic) and include that computed path in the thrown Error message so the failure message accurately reflects non-default Shakapacker layouts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@client/app/bundles/comments/components/SimpleCommentScreen/ror_components/SimpleCommentScreen.jsx`:
- Line 3: ESLint is still failing because the single-line disable at the top
doesn't cover both class declarations in SimpleCommentScreen.jsx; either add a
file-level directive (/* eslint-disable max-classes-per-file */) at the very top
of SimpleCommentScreen.jsx so it applies to all class declarations in the file,
or move one of the class declarations into its own module and import it (i.e.,
split the two class declarations into separate files) and remove the inline
disable; locate the two class declarations in SimpleCommentScreen.jsx and choose
one of these fixes to resolve the rule failure.
In
`@client/app/bundles/comments/startup/RouterApp/ror_components/RouterApp.server.jsx`:
- Line 1: Remove the top-level 'use client' directive from RouterApp.server.jsx
so the module is treated as a server component (it currently imports server-only
APIs like react-router-dom/server); open the file and delete the "'use client'"
line and ensure no client-only hooks or APIs are used elsewhere in the module
(check for references inside RouterApp or any exported server functions) so the
import of react-router-dom/server remains valid.
In `@client/app/bundles/server-components/components/CommentsFeed.jsx`:
- Around line 19-21: In CommentsFeed.jsx replace the hardcoded
'http://localhost:3000/comments.json' fetch with an environment-aware or
relative URL (e.g., use a relative path '/comments.json' or a runtime config
variable like process.env.NEXT_PUBLIC_API_URL) and add response status handling
before parsing: check response.ok in the function that calls fetch and throw or
handle an error when it's false so you don't call response.json() on error
responses; adjust any surrounding error-handling logic in the CommentsFeed
component to surface or recover from that thrown error.
- Line 16: The Promise executor in CommentsFeed.jsx uses an implicit-return
arrow that returns setTimeout (await new Promise((resolve) =>
setTimeout(resolve, 800))), triggering the no-promise-executor-return lint rule;
change the executor to a block-bodied function and call setTimeout inside it
(i.e., replace the inline implicit-return with a brace-enclosed function that
invokes setTimeout and calls resolve), so the Promise executor does not return
the timer value.
In `@client/app/bundles/server-components/components/ServerInfo.jsx`:
- Around line 43-45: Replace the unstable array-index key used in the
grouped.map call (key={gi}) with a stable identifier derived from the group's
contents: inside the ServerInfo component replace the gi key with a
deterministic value (for example build a key from the group's item keys like
group.map(([k])=>k).join('-') or use the first item's key group[0][0] if
present). Update the grouped.map(...) element (where gi is currently used) to
use this computed stableKey so React reconciliation is stable and lint warnings
are resolved.
In `@client/app/bundles/server-components/components/TogglePanel.jsx`:
- Around line 5-34: Import PropTypes from 'prop-types' and add propTypes on the
TogglePanel component to satisfy the react/prop-types lint rule: add
"TogglePanel.propTypes = { title: PropTypes.node.isRequired, children:
PropTypes.node }" (and optionally defaultProps if desired). This change should
be made near the TogglePanel definition (reference: TogglePanel component, props
title and children) and ensure the PropTypes import is included at the top of
the file.
In `@client/app/packs/rsc-client-components.js`:
- Around line 7-8: The ESLint import/order violation is caused by importing the
relative module TogglePanel before the external package ReactOnRails; fix it by
reordering the imports so ReactOnRails is imported first and TogglePanel second
(i.e., move the ReactOnRails import above the TogglePanel import in
rsc-client-components.js), then re-run the linter to confirm the import/order
rule passes.
In `@config/webpack/rscWebpackConfig.js`:
- Around line 65-66: The guard that checks for url/file loaders in
rscWebpackConfig.js is too strict (it compares rule.use.loader using ===), so
emitFile=false is not applied when loader identifiers are resolved to full
paths; update the condition in the block that sets rule.use.options.emitFile to
use substring matching (e.g., use .includes('url-loader') ||
.includes('file-loader')) or call the existing extractLoader() helper to detect
loader identity, ensuring the check matches resolved loader paths and reliably
sets emitFile=false for the url-loader/file-loader cases.
In `@config/webpack/rspackRscPlugin.js`:
- Around line 12-35: The module-level cache useClientCache can become stale
during watch/dev builds; update the plugin's apply method to clear
useClientCache at the start of each compilation (e.g., register a hook on
compiler.hooks.compilation or compiler.hooks.watchRun depending on the plugin
entry) so hasUseClientDirective always re-reads changed files between rebuilds;
locate useClientCache and hasUseClientDirective and add the cache.clear() call
inside the apply's compilation/watch hook to invalidate cached results on each
new build.
In `@config/webpack/webpackConfig.js`:
- Around line 23-33: The RSC-only branch never applies env-specific adjustments
and doesn't expose the rsc config for modification; update the RSC_BUNDLE_ONLY
branch so after creating rscConfig via rscWebpackConfig() you call envSpecific
appropriately (e.g., envSpecific(null, null, rscConfig) or extend envSpecific to
accept a third rscConfig arg) before assigning result = rscConfig, ensuring
rscConfig is passed to envSpecific exactly like clientConfig and serverConfig
are in the multi-config path (symbols: rscWebpackConfig, envSpecific,
clientWebpackConfig, serverWebpackConfig, result).
---
Nitpick comments:
In `@app/controllers/pages_controller.rb`:
- Line 41: The new controller action server_components is being run through the
before_action :set_comments even though it does not use `@comments`; update the
before_action declaration to skip set_comments for this action by adding an
except/only exemption (e.g., modify the before_action :set_comments line to
exclude :server_components) so the database work is avoided when
server_components is invoked.
In `@app/views/pages/server_components.html.erb`:
- Around line 2-4: The react_component call currently hardcodes trace: true;
change it to enable tracing only in development by replacing the literal with a
conditional using the Rails environment (e.g., set trace to
Rails.env.development? or equivalent), updating the
react_component("ServerComponentsPage", prerender: false, trace: ...) invocation
so trace is true only when Rails.env.development? and false otherwise.
In `@client/app/bundles/server-components/components/TogglePanel.jsx`:
- Around line 10-29: The TogglePanel component's toggle button needs ARIA
attributes so assistive tech can read its state: update the button (where
setIsOpen is used and isOpen is referenced) to include aria-expanded={isOpen}
and aria-controls pointing to the panel's id, and give the panel div (the
conditional container that renders {children}) a matching id; implement the id
either by accepting a prop (e.g., panelId) or generating a stable id inside the
component (e.g., React's useId) and use that id on the div so aria-controls
links to it.
In `@config/webpack/commonWebpackConfig.js`:
- Around line 11-17: The current webpack alias object only maps exact imports
('react-on-rails$' and 'react-on-rails/node_package') and misses other subpath
imports; update the alias entries in the alias object to use a regex/broader
pattern so any import of react-on-rails and its subpaths (e.g.,
react-on-rails/... ) is redirected to react-on-rails-pro and its corresponding
subpath, replacing or supplementing the existing keys in the alias object to
ensure complete coverage across imports; test subpath resolution behavior in
your webpack/rspack version after changing the alias.
In `@config/webpack/rscWebpackConfig.js`:
- Around line 89-90: The code currently overwrites the inherited optimization
object (rscConfig.optimization = { minimize: false }), dropping settings
established by commonWebpackConfig(); instead merge into the existing object by
preserving rscConfig.optimization and only changing minimize (e.g., assign
rscConfig.optimization = { ...rscConfig.optimization, minimize: false } or set
rscConfig.optimization.minimize = false) so other optimization defaults remain
intact; update the rscWebpackConfig code that sets rscConfig.optimization
accordingly.
- Around line 29-33: The error thrown when rscEntry is missing should mirror the
server-bundle diagnostics by deriving the expected RSC bundle path from the same
config values instead of hard-coding "client/app/packs/rsc-bundle.js"; update
the check around rscEntry (rscEntry and rscConfig.entry) to compute the expected
path using config.source_path and config.source_entry_path (same approach used
in serverWebpackConfig's bundle path logic) and include that computed path in
the thrown Error message so the failure message accurately reflects non-default
Shakapacker layouts.
In `@config/webpack/rspackRscPlugin.js`:
- Around line 20-24: The current synchronous file reads (fs.openSync,
fs.readSync, fs.closeSync) in the RSC scanning logic block that reads the first
~200 bytes of filePath block the event loop and can degrade large builds; change
this to use fs.promises (e.g., fs.promises.open andfilehandle.read or
fs.promises.readFile with a slice) and perform reads concurrently with
Promise.all so many files are scanned in parallel, and update the surrounding
processAssets hook (or the function that calls this scan) to be
async/await-compatible so it can await the promise-based reads without blocking;
keep the existing cache semantics and only switch the low-level I/O to the
promise-based APIs while preserving the logic that checks for 'use client' at
the top of filePath.
In `@package.json`:
- Line 132: Update the eslint-plugin-react-hooks dependency in package.json to
the latest compatible release (replace the "eslint-plugin-react-hooks": "^4.6.0"
entry) so the project uses the React 19–compatible validator (e.g., 7.0.1);
modify the package.json dependency line for eslint-plugin-react-hooks and then
run your package manager (npm/yarn/pnpm) to install and update lockfiles, and
run the linter to confirm no new rule errors from the updated plugin.
🪄 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: 362cf633-c476-4648-9bdf-6c7c981ca0ac
⛔ Files ignored due to path filters (2)
Gemfile.lockis excluded by!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (33)
GemfileProcfile.devapp/controllers/pages_controller.rbapp/views/pages/server_components.html.erbclient/app/bundles/comments/components/Footer/ror_components/Footer.jsxclient/app/bundles/comments/components/NavigationBar/NavigationBar.jsxclient/app/bundles/comments/components/SimpleCommentScreen/ror_components/SimpleCommentScreen.jsxclient/app/bundles/comments/constants/paths.jsclient/app/bundles/comments/rescript/ReScriptShow/ror_components/RescriptShow.jsxclient/app/bundles/comments/startup/App/ror_components/App.jsxclient/app/bundles/comments/startup/ClientRouterAppExpress.jsxclient/app/bundles/comments/startup/NavigationBarApp/ror_components/NavigationBarApp.jsxclient/app/bundles/comments/startup/RouterApp/ror_components/RouterApp.client.jsxclient/app/bundles/comments/startup/RouterApp/ror_components/RouterApp.server.jsxclient/app/bundles/comments/startup/serverRegistration.jsxclient/app/bundles/server-components/ServerComponentsPage.jsxclient/app/bundles/server-components/components/CommentsFeed.jsxclient/app/bundles/server-components/components/ServerInfo.jsxclient/app/bundles/server-components/components/TogglePanel.jsxclient/app/libs/requestsManager.jsclient/app/packs/rsc-bundle.jsclient/app/packs/rsc-client-components.jsclient/app/packs/stimulus-bundle.jsclient/app/packs/stores-registration.jsconfig/initializers/react_on_rails_pro.rbconfig/routes.rbconfig/webpack/clientWebpackConfig.jsconfig/webpack/commonWebpackConfig.jsconfig/webpack/rscWebpackConfig.jsconfig/webpack/rspackRscPlugin.jsconfig/webpack/serverWebpackConfig.jsconfig/webpack/webpackConfig.jspackage.json
...t/app/bundles/comments/components/SimpleCommentScreen/ror_components/SimpleCommentScreen.jsx
Outdated
Show resolved
Hide resolved
client/app/bundles/comments/startup/RouterApp/ror_components/RouterApp.server.jsx
Show resolved
Hide resolved
client/app/bundles/server-components/components/CommentsFeed.jsx
Outdated
Show resolved
Hide resolved
client/app/bundles/server-components/components/CommentsFeed.jsx
Outdated
Show resolved
Hide resolved
client/app/bundles/server-components/components/CommentsFeed.jsx
Outdated
Show resolved
Hide resolved
client/app/bundles/server-components/components/CommentsFeed.jsx
Outdated
Show resolved
Hide resolved
client/app/bundles/server-components/components/CommentsFeed.jsx
Outdated
Show resolved
Hide resolved
client/app/bundles/server-components/components/CommentsFeed.jsx
Outdated
Show resolved
Hide resolved
Review: Add React Server Components with React on Rails ProSolid implementation overall — the three-bundle pipeline, custom Security
Correctness
Code Quality
🤖 Generated with Claude Code |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f7e452b77
ℹ️ 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".
…ation Three root causes for the 37/38 rspec test failures: 1. CI missing Node renderer: The RSC branch switched SSR from ExecJS to the react-on-rails-pro NodeRenderer service (port 3800). CI never started this service, causing Net::ReadTimeout on all SSR requests. Added renderer startup step and RENDERER_PASSWORD env var to CI. 2. Server bundle externals broke in VM sandbox: The previous commit externalized Node builtins (path/fs/stream) as CommonJS requires, but the Node renderer runs bundles in a vm.createContext() sandbox where require() is unavailable. Reverted to resolve.fallback: false which stubs these unused code paths at build time instead. 3. MessageChannel undefined in VM: react-dom/server.browser.js instantiates MessageChannel at module load time. The Node renderer's VM sandbox lacks this browser global (unlike Bun/ExecJS on master). Added a BannerPlugin polyfill injected at bundle top. 4. RouterApp.server.jsx misclassified as RSC: The auto-bundling system registered it via registerServerComponent() because it lacked 'use client'. But it's a traditional SSR component (StaticRouter), not an RSC. Added 'use client' directive so it registers via ReactOnRails.register() instead. All 38 rspec tests now pass locally. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
client/app/bundles/server-components/components/CommentsFeed.jsx
Outdated
Show resolved
Hide resolved
client/app/bundles/server-components/components/CommentsFeed.jsx
Outdated
Show resolved
Hide resolved
client/app/bundles/comments/startup/RouterApp/ror_components/RouterApp.server.jsx
Show resolved
Hide resolved
|
Review: React Server Components with React on Rails Pro Good work overall - the three-bundle pipeline is well-structured and the RSC demo clearly illustrates the server/client component model. A few issues need attention before merging. Security (must fix)
Code quality
Minor
|
Review follow-up completed in
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
There are 3 total unresolved issues (including 1 from previous review).
Bugbot Autofix is ON, but it could not run because on-demand usage is turned off. To enable Bugbot Autofix, turn on on-demand usage and set a spend limit in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit bc41706. Configure here.
| sleep 1 | ||
| done | ||
| echo "Node renderer failed to start within 30 seconds" | ||
| exit 1 |
There was a problem hiding this comment.
CI starts node renderer but Rails won't use it
Medium Severity
The CI workflow starts the node renderer process and waits for it, but the Rails initializer only enables the node renderer when Rails.env.development? or ENV["REACT_USE_NODE_RENDERER"] == "true". In CI, RAILS_ENV=test so development? is false, and REACT_USE_NODE_RENDERER is never set. The renderer starts but Rails never connects to it, meaning any test relying on RSC or node-based SSR will silently fall back to non-node rendering.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit bc41706. Configure here.
| const testValue = typeof item === 'string' ? item : item?.loader; | ||
| return testValue && testValue.includes(loaderName); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Duplicated extractLoader function across webpack config files
Low Severity
extractLoader is defined identically in both rscWebpackConfig.js and serverWebpackConfig.js. The server config already exports it via module.exports.extractLoader = extractLoader, but the RSC config defines its own copy instead of importing it. This duplication risks divergent fixes if the function is ever updated.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit bc41706. Configure here.
PR Review: Add React Server Components with React on Rails ProThis is an impressive and well-structured PR that adds RSC support via a clean three-bundle pipeline. The custom Security
Correctness / Bugs
Code Quality
|
| DRIVER: selenium_chrome | ||
| CHROME_BIN: /usr/bin/google-chrome | ||
| USE_COVERALLS: true | ||
| RENDERER_PASSWORD: devPassword |
There was a problem hiding this comment.
Weak password hardcoded in version control. Use a GitHub Actions secret instead:
| RENDERER_PASSWORD: devPassword | |
| RENDERER_PASSWORD: ${{ secrets.RENDERER_PASSWORD }} |
Add a fallback (e.g. devPassword) only in the secret's default value, not in the YAML.
| freeMemory: (os.freemem() / (1024 * 1024 * 1024)).toFixed(1), | ||
| cpus: os.cpus().length, | ||
| hostname: os.hostname(), | ||
| }; |
There was a problem hiding this comment.
Security: hostname disclosure. Exposing the server's hostname to any page visitor is useful reconnaissance for an attacker. If this is a public demo page, consider omitting the hostname or replacing it with a masked value (e.g. os.hostname().replace(/[a-z0-9]+/gi, '*')).
| img: ['src', 'alt', 'title', 'width', 'height'], | ||
| }, | ||
| allowedSchemes: ['https', 'http'], | ||
| }); |
There was a problem hiding this comment.
Security: http:// allows mixed-content and IP-leak via <img> tags. User-submitted markdown can contain  which will fire a request from the server (during RSC render) or from the browser (after hydration), leaking the server/visitor IP. Restrict to HTTPS only:
| }); | |
| allowedSchemes: ['https'], |
| @@ -1,8 +1,10 @@ | |||
| 'use client'; | |||
There was a problem hiding this comment.
Misleading 'use client' directive. This is a traditional SSR component that uses StaticRouter and is only included in the server bundle — the RSC bundle explicitly excludes it (see the comment in rsc-bundle.js). The 'use client' directive here is dead code that contradicts the filename and could mislead future maintainers into thinking this file participates in the RSC graph. Remove the directive, or add a comment explaining why it is intentionally present.
| const testValue = typeof item === 'string' ? item : item?.loader; | ||
| return testValue && testValue.includes(loaderName); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Duplicate function. extractLoader is already defined and exported from serverWebpackConfig.js. Import it instead of re-defining it:
| } | |
| const { extractLoader } = require('./serverWebpackConfig'); |
| manifest[key] = {}; | ||
| } | ||
| manifest[key]['*'] = ssrEntry; | ||
| manifest[key][''] = ssrEntry; |
There was a problem hiding this comment.
Likely incorrect chunks format. chunks.push(chunk.id, file) pushes two separate elements per file, producing a flat alternating array like [chunkId, "file.js", chunkId, "file2.js"]. The React Flight manifest format expects chunks to be an array of filenames (strings), not an interleaved id/name array. Compare against what RSCWebpackPlugin emits — the format is typically ["file.js"]. If chunk IDs are needed, they belong in a separate field or as [id, file] tuples depending on the React version.
|
✅ Review app for PR #722 was successfully deleted |
1 participant
Summary
react_on_railstoreact_on_rails_pro(16.5.1) with React 19.0.4 andreact-on-rails-rsc/server-componentsshowcasing server components with async data fetching, Suspense streaming, and interactive client componentsRspackRscPluginfor manifest generation since the standardRSCWebpackPluginuses webpack internals incompatible with RspackRSC Demo Features
osmodule andlodash(never shipped to client browser)Suspensestreaming — comments fetched directly from Rails API on server'use client'TogglePanel) mixed with server-rendered content (donut pattern)marked+sanitize-html(~200KB of libraries that stay server-side)Infrastructure Changes
rscWebpackConfig.js— RSC bundle targeting Node.js withreact-on-rails-rsc/WebpackLoaderrspackRscPlugin.js— Lightweight Rspack-compatible alternative to the webpack-onlyRSCWebpackPlugin'use client'directives added to all existing client component entry pointsreact-on-rails→react-on-rails-profor third-party compatibilityrsc-bundle.jsentry withregisterServerComponentserver/client registrationrsc_payload_routeadded to routes for RSC payload deliveryTest plan
spec/requests/server_render_check_spec.rb)/server-componentsto verify RSC demo page renders/rsc_payload/ServerComponentsPage🤖 Generated with Claude Code
Note
High Risk
High risk because it changes the React on Rails integration and SSR execution model (introduces a Node renderer, new RSC payload route, and a third build output), which can break rendering/builds and affects server-side execution paths.
Overview
Adds React Server Components (RSC) support by migrating from
react_on_railstoreact_on_rails_proand introducing a Node-based renderer process used for SSR and RSC payload generation.Introduces a third build target (
rsc-bundle) alongside client and SSR bundles, including a customRspackRscPluginto emit React Flight manifests and updates to webpack configs (aliases, fallbacks for Node builtins, Node-targeted RSC config, and SSR outputcommonjs2).Adds an
/server-componentsdemo page wired into Rails routes/controller/views and the nav, plus new RSC client/server pack entrypoints and dev/CI plumbing (Procfile, npm script, CI step/env) to start the renderer and build the additional bundles.Reviewed by Cursor Bugbot for commit bc41706. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Chores