Upgrade React on Rails/Shakapacker and standardize on Rspack v2#702
Upgrade React on Rails/Shakapacker and standardize on Rspack v2#702
Conversation
|
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:
WalkthroughMigrates the project from Webpack to Rspack: bumps dependencies, updates Procfiles/README, enforces Rspack-only bundler behavior via refactored bundler utils and config changes, updates tests, adds a .claude command doc, adjusts .gitignore, and bumps Node version in a Dockerfile. (49 words) Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
🚀 Quick Review App CommandsWelcome! Here are the commands you can use in this PR:
|
Greptile SummaryThis PR promotes the tutorial project from a dual webpack/Rspack setup to a standardised Rspack-only stack: it promotes Key observations:
Confidence Score: 4/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["shakapacker.yml\nassets_bundler: rspack"] --> B["bundlerUtils.js\nensureRspack()"]
B -- "=== rspack" --> C["require @rspack/core\ncached in _cachedBundler"]
B -- "!== rspack" --> D["throw Error\nconfigured for Rspack only"]
C --> E["getBundler()"]
C --> F["getCssExtractPlugin()\nCssExtractRspackPlugin"]
E --> H["client.js\nnew bundler.ProvidePlugin"]
F --> I["CSS extraction in webpack configs"]
A --> G["isRspack()\nreturns true"]
K["@rspack/plugin-react-refresh"] --> L["Shakapacker handles HMR\nno extra wiring in development.js"]
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
PR Review: Upgrade React on Rails/Shakapacker and standardize on Rspack v2Overall: The direction is good - promoting Rspack to the sole supported bundler and upgrading all libraries to stable releases. A few code issues worth addressing before merging. Issue 1: Dead code in config/webpack/client.js Lines 7-9 still use the old dual-bundler detection with a require webpack fallback, bypassing bundlerUtils.getBundler() enforcement. If assets_bundler is misconfigured, this silently loads webpack instead of throwing the clear error message introduced in bundlerUtils.js. Lines 26-28 are an always-false empty block - since rspack is the only supported bundler, the condition Issue 2: Dead code in config/webpack/development.js The Issue 3: Inconsistency between getBundler() and isRspack() in bundlerUtils.js
Issue 4: Pinned prerelease for @rspack/core and @rspack/cli Pinning to |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
🎉 ✨ Deploy Complete! 🚀🌐 ➡️ Open Review AppDeployment successful for PR #702, commit 5945fe5 🎮 Control Plane Console |
🚀 Deploying to Control Plane...⏳ Waiting for deployment to be ready... |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
config/webpack/client.js (1)
7-9: 🛠️ Refactor suggestion | 🟠 MajorRoute both
client.jsandserver.jsthroughgetBundler()for consistent bundler resolution.Both
config/webpack/client.js(lines 8-9) andconfig/webpack/server.js(lines 9-10) have direct bundler requires that bypass the centralizedconfig/webpack/bundlerUtils.jsguard. The sharedgetBundler()path now owns the Rspack-only contract, but these entry points still resolve the bundler directly. Refactor both files to usegetBundler()instead to enforce a single source of truth.♻️ Proposed cleanup for client.js
const environment = require('./environment'); +const { getBundler } = require('./bundlerUtils'); -const bundler = config.assets_bundler === 'rspack' - ? require('@rspack/core') - : require('webpack'); +const bundler = getBundler();Apply the same pattern to
server.js.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/webpack/client.js` around lines 7 - 9, The client and server entrypoints currently bypass the centralized bundler resolution by directly requiring webpack/rspack; import and use the getBundler() helper from config/webpack/bundlerUtils instead. Replace the existing ternary/require logic that assigns bundler with: add an import/require for getBundler and set const bundler = getBundler(), and remove the direct require('@rspack/core')/require('webpack') code in both client.js and server.js so bundler resolution is centralized through getBundler().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Procfile.dev-prod-assets`:
- Line 10: The watcher command in Procfile.dev-prod-assets calls
`react_on_rails:locale && rm -rf public/packs/* || true && bin/shakapacker -w`,
and the `|| true` currently masks failures from `react_on_rails:locale` so the
watcher (`bin/shakapacker -w`) still starts on locale errors; update the command
so that failures in `react_on_rails:locale` are not ignored (either remove the
`|| true` entirely or move it to only apply to `rm -rf public/packs/*`),
ensuring `react_on_rails:locale` runs with `&&` before starting `bin/shakapacker
-w` and that the watcher only starts when locale/build prep succeeds.
In `@Procfile.dev-static`:
- Line 11: The current rspack command chain lets the watcher start even if
`bundle exec rake react_on_rails:locale` fails because of the `|| true`; update
the command so rake failures abort startup but still tolerate `rm -rf
public/packs/*` errors. Concretely, remove the `|| true` after the rake step and
instead isolate the cleanup with a tolerant grouping such as wrapping `rm -rf
public/packs/*` in a subshell or grouping with its own `|| true` (e.g. `(rm -rf
public/packs/* || true)`), keeping `&& bin/shakapacker -w` so `bin/shakapacker
-w` only runs if the rake step succeeds; target the rspack Procfile dev entry
containing `bundle exec rake react_on_rails:locale`, `rm -rf public/packs/*`,
and `bin/shakapacker -w`.
In `@Procfile.dev-static-assets`:
- Line 10: The rspack process command currently masks failures by using "&& rm
-rf public/packs/* || true && bin/shakapacker -w" so bin/shakapacker -w runs
even if react_on_rails:locale fails; remove the "|| true" and ensure the command
is strictly chained with "&&" (i.e., run "bundle exec rake react_on_rails:locale
&& rm -rf public/packs/* && bin/shakapacker -w") so that a failing
react_on_rails:locale prevents the watcher (bin/shakapacker -w) from starting.
In `@README.md`:
- Line 33: Update the bullet text that reads "Optimizing your front end setup
with Rspack + Shakapacker for React on Rails, including SSR and code splitting."
to use the hyphenated compound adjective "front‑end setup" (i.e., change "front
end setup" to "front-end setup") so the sentence reads "Optimizing your
front-end setup with Rspack + Shakapacker for React on Rails, including SSR and
code splitting."
---
Outside diff comments:
In `@config/webpack/client.js`:
- Around line 7-9: The client and server entrypoints currently bypass the
centralized bundler resolution by directly requiring webpack/rspack; import and
use the getBundler() helper from config/webpack/bundlerUtils instead. Replace
the existing ternary/require logic that assigns bundler with: add an
import/require for getBundler and set const bundler = getBundler(), and remove
the direct require('@rspack/core')/require('webpack') code in both client.js and
server.js so bundler resolution is centralized through getBundler().
🪄 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: c9a6d853-4ace-410b-8057-737d07e5d723
⛔ Files ignored due to path filters (2)
Gemfile.lockis excluded by!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (11)
GemfileProcfile.devProcfile.dev-prod-assetsProcfile.dev-staticProcfile.dev-static-assetsREADME.mdclient/__tests__/webpack/bundlerUtils.spec.jsconfig/webpack/bundlerUtils.jsconfig/webpack/client.jsconfig/webpack/development.jspackage.json
PR Review: Upgrade React on Rails/Shakapacker and standardize on Rspack v2Overall this is a clean upgrade graduating react_on_rails and shakapacker from RC to stable. A few issues worth addressing: Bugs / Code Quality1. config/webpack/client.js bypasses the Rspack enforcement guard client.js has its own inline bundler detection (lines 7-9) that silently falls back to requiring webpack instead of calling getBundler() from bundlerUtils.js. If someone sets assets_bundler: webpack in shakapacker.yml, client.js loads webpack silently while bundlerUtils.js would throw. The enforcement is inconsistent — this file should use getBundler() from bundlerUtils.js instead. 2. Dead empty if block in config/webpack/client.js (lines 26-28) The condition 3. Dead empty if block in config/webpack/development.js (lines 11-14) The Dependency Risk4. @rspack/core 2.0.0-beta.7 is a prerelease Since this is a tutorial/reference repo that many developers clone as a starting point, consider adding a visible callout in the README near the version targets table so readers are not caught off-guard by breaking changes as Rspack 2.x stabilizes. Minor / Documentation5. Heroku link URL still targets Rails 7 The display text was updated but the href still points to 6. isRspack() semantics are misleading in an rspack-only repo isRspack() returns false for non-rspack bundlers rather than throwing, while getBundler() throws. Any caller that branches on isRspack() === false to reach a webpack fallback is now silently wrong. Consider removing isRspack() or aligning it to throw on non-rspack configs like the other helpers. What is good
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: React Fast Refresh plugin installed but never configured
- Added
@rspack/plugin-react-refreshtoconfig/webpack/development.jsand push it into the client plugins list when development HMR styling is enabled.
- Added
- ✅ Fixed: Empty conditional block is dead code
- Removed the empty
if (devBuild && isHMR && config.assets_bundler !== 'rspack')no-op block fromconfig/webpack/client.js.
- Removed the empty
Or push these changes by commenting:
@cursor push 62ada50be9
Preview (62ada50be9)
diff --git a/config/webpack/client.js b/config/webpack/client.js
--- a/config/webpack/client.js
+++ b/config/webpack/client.js
@@ -23,8 +23,4 @@
}),
);
-if (devBuild && isHMR && config.assets_bundler !== 'rspack') {
- // Rspack is the only supported bundler for this repo; no webpack refresh plugin wiring.
-}
-
module.exports = environment;
diff --git a/config/webpack/development.js b/config/webpack/development.js
--- a/config/webpack/development.js
+++ b/config/webpack/development.js
@@ -4,13 +4,13 @@
process.env.NODE_ENV = process.env.NODE_ENV || 'development';
const { inliningCss } = require('shakapacker');
+const ReactRefreshRspackPlugin = require('@rspack/plugin-react-refresh');
const webpackConfig = require('./webpackConfig');
-const developmentEnvOnly = (_clientWebpackConfig, _serverWebpackConfig) => {
+const developmentEnvOnly = (clientWebpackConfig, _serverWebpackConfig) => {
if (inliningCss) {
- // Rspack HMR/refresh is handled by Shakapacker + @rspack/plugin-react-refresh.
- // No extra development-only plugin wiring is needed here.
+ clientWebpackConfig.plugins.push(new ReactRefreshRspackPlugin());
}
};
🎉 ✨ Deploy Complete! 🚀🌐 ➡️ Open Review AppDeployment successful for PR #702, commit 5945fe5 🎮 Control Plane Console |
🎉 ✨ Deploy Complete! 🚀🌐 ➡️ Open Review AppDeployment successful for PR #702, commit 29ad680 🎮 Control Plane Console |
Review SummaryOverall this is a clean, well-scoped upgrade. The gem/npm versions are now at stable releases (dropping the One actual bug (3 files)
Minor inconsistency
Noted (no action needed)
|
🎉 ✨ Deploy Complete! 🚀🌐 ➡️ Open Review AppDeployment successful for PR #702, commit b22539a 🎮 Control Plane Console |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Around line 199-201: Update the README entry that lists bundler files so the
filename matches the codebase: replace the referenced "webpack.config.js" with
"webpackConfig.js" (as required by files like config/webpack/development.js
which do require('./webpackConfig')). Edit the README section that currently
names webpack.config.js and ensure the list reads "webpackConfig.js - Main
Shakapacker entry point" to avoid confusion.
🪄 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: e1a10280-ac9e-4c09-8a3b-726b93d396df
📒 Files selected for processing (5)
README.mdclient/__tests__/webpack/bundlerUtils.spec.jsconfig/webpack/bundlerUtils.jsconfig/webpack/client.jsconfig/webpack/development.js
🚧 Files skipped from review as they are similar to previous changes (2)
- client/tests/webpack/bundlerUtils.spec.js
- config/webpack/client.js
🎉 ✨ Deploy Complete! 🚀🌐 ➡️ Open Review AppDeployment successful for PR #702, commit a7f8094 🎮 Control Plane Console |
PR Review: Upgrade React on Rails/Shakapacker + Rspack v2Overall this is a clean, well-scoped upgrade. The dependency bumps to stable A few issues worth addressing before merge: Build/Deployment Risk
Orphaned webpack packages — Correctness / Fragility Plugin filter by constructor name in
Prerelease dependency
Minor
|
🎉 ✨ Deploy Complete! 🚀🌐 ➡️ Open Review AppDeployment successful for PR #702, commit e23c883 🎮 Control Plane Console |
Review: Upgrade React on Rails/Shakapacker and standardize on Rspack v2Overall this is a clean, well-scoped upgrade. The move from RC versions to stable
|
| "@rspack/cli": "2.0.0-beta.7", | ||
| "@rspack/core": "2.0.0-beta.7", |
There was a problem hiding this comment.
Pinning to a pre-RC beta (2.0.0-beta.7) in a tutorial that serves as the canonical React on Rails example is risky. Semver pre-release conventions allow breaking changes between beta iterations, so a fresh yarn install after any upstream beta bump could break the tutorial for new users.
If v2 is needed before a stable/RC is available, consider using a range like >=2.0.0-beta.7 <2.0.0 to get patch-level beta fixes while staying on this major, and add a README callout that the v2 dependency is pre-release.
| * @returns {boolean} True if using Rspack, false if using Webpack | ||
| * @returns {boolean} True when assets_bundler is rspack | ||
| */ | ||
| const isRspack = () => config.assets_bundler === 'rspack'; |
There was a problem hiding this comment.
isRspack() is now redundant given the Rspack-only enforcement in getBundler(). In any valid configuration it always returns true; in an invalid one, the next getBundler() call throws anyway. Keeping it exported implies a non-Rspack path still exists, which could mislead future contributors. Consider removing it (and its export) — call sites can simply use true or be removed.
| @@ -1,16 +1,17 @@ | |||
| const devBuild = process.env.NODE_ENV === 'development'; | |||
| // Set by shakapacker dev-server for both webpack and rspack. | |||
| const isHMR = process.env.WEBPACK_DEV_SERVER === 'TRUE'; | |||
There was a problem hiding this comment.
The variable name isHMR and env var WEBPACK_DEV_SERVER are both Webpack-flavored while the project is now Rspack-only. The comment accurately notes that Shakapacker sets this env var for both bundlers, which is reassuring, but a new reader may be confused by the mismatch. A follow-up rename to isDevServer (or similar) would reduce friction for tutorial readers.
|
✅ Review app for PR #702 was successfully deleted |
![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: e23c8832f6
ℹ️ 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".
|
|
||
| ## Javascript Development without Rails using the Rspack Dev Server | ||
|
|
||
| Start the full development stack with `foreman start -f Procfile.dev`, then open <http://localhost:4000> to iterate on the JavaScript client with hot reloading. |
There was a problem hiding this comment.
Following this new quick-start now sends contributors to localhost:4000, but Procfile.dev only starts Rails on port 3000 and the Shakapacker dev server is configured for port 3035 in config/shakapacker.yml. There is no service bound to 4000 anywhere in the repo, so a fresh setup appears broken even when both processes started correctly.
Useful? React with 👍 / 👎.
…#728) * Switch SSR to the Pro Node renderer on webpack Second of three stacked sub-PRs in the Pro RSC migration. Routes all server rendering through the Pro Node renderer (port 3800) instead of ExecJS, and flips the asset bundler from rspack to webpack — scoped reversal of #702, needed because rspack 2.0.0-beta.7's webpack compatibility layer doesn't cover the APIs upstream RSCWebpackPlugin requires. We flip back to rspack once shakacode/react_on_rails_rsc#29 ships a native rspack RSC plugin. The bundler flip and NodeRenderer wiring ship atomically: the server bundle produced by the Pro webpack transforms (target: 'node' + libraryTarget: 'commonjs2') is not evaluable by ExecJS, so the initializer pointing server_renderer at the NodeRenderer must land at the same time. Key changes: - config/shakapacker.yml: assets_bundler: rspack → webpack - config/webpack/bundlerUtils.js: return @rspack/core or webpack based on the shakapacker setting (was rspack-only and threw otherwise); spec updated in parallel - config/webpack/serverWebpackConfig.js: Pro transforms per the :pro generator's update_webpack_config_for_pro and the marketplace/dummy references — target: 'node' + node: false, libraryTarget: 'commonjs2', extractLoader helper, babelLoader.options.caller = { ssr: true }, destructured module.exports so Sub-PR 3's rscWebpackConfig.js can derive from serverWebpackConfig(true). RSCWebpackPlugin({ isServer: true }) when !rscBundle emits the server manifest; inert until Sub-PR 3 activates RSC support - config/initializers/react_on_rails_pro.rb: NodeRenderer-only config (no RSC fields — those move in Sub-PR 3) - renderer/node-renderer.js: launcher with strict integer env parsing, CI worker cap, and additionalContext: { URL, AbortController } so react-router-dom's NavLink.encodeLocation does not throw "ReferenceError: URL is not defined" at SSR time - Procfile.dev: renderer: NODE_ENV=development node renderer/node-renderer.js - package.json: react-on-rails-pro-node-renderer 16.6.0 and react-on-rails-rsc ^19.0.4 (Pro peer dep; required for the RSCWebpackPlugin import) - .gitignore: /renderer/.node-renderer-bundles/ - .env.example: REACT_ON_RAILS_PRO_LICENSE, RENDERER_PASSWORD, and REACT_RENDERER_URL with dev vs prod guidance - .github/workflows/rspec_test.yml: start the Node renderer before rspec with PID liveness and port-ready checks plus log capture on failure Verified locally: webpack build compiles cleanly. `bin/rails s` on 3000 with `node renderer/node-renderer.js` on 3800 renders GET / at HTTP 200; Rails log shows "Node Renderer responded" and the renderer log emits "[SERVER] RENDERED Footer to dom node with id: ..." — confirming SSR went through the Pro path rather than falling back to ExecJS. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Tighten Pro webpack + initializer setup from reference audit Four fixes from auditing Sub-PR 2 against the Pro dummy, the :pro generator, and the Pro configuration docs. - config/initializers/react_on_rails_pro.rb: renderer_password now raises in non-local envs instead of falling back to the dev string. Previously any production deploy that forgot to set the env var would silently run with a known-public password; now it fails loudly. Matches the safer pattern from PR #723's final state. - config/webpack/serverWebpackConfig.js: pass clientReferences to RSCWebpackPlugin({ isServer: true }), matching the Pro dummy's serverWebpackConfig at `react_on_rails_pro/spec/dummy/config/webpack/ serverWebpackConfig.js`. Without it, the plugin may walk into node_modules and hit unlodaed .tsx source files and re-scan modules we don't need. Locks client-ref discovery to client/app/**. - config/webpack/serverWebpackConfig.js: drop publicPath from the server-bundle output. Server bundles are loaded by the Node renderer via the filesystem, never served over HTTP — the URL is unused. Matches the Pro dummy's comment. - package.json: pin react-on-rails-rsc to 19.0.4 stable (was ^19.0.4 range) and add "node-renderer" npm script as a convenience shortcut for `node renderer/node-renderer.js`. - .github/workflows/rspec_test.yml: set RENDERER_PASSWORD explicitly in CI to the shared dev default so both the initializer and the launcher use the same concrete value, avoiding silent drift if either side's default is ever touched. Re-verified: webpack build clean; renderer + Rails boot; GET / returns HTTP 200 with "Node Renderer responded" in the Rails log and "[SERVER] RENDERED" in the renderer log, confirming SSR still goes through the Pro path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Align renderer_password initializer with Pro docs + source Previous commit added Rails.env-branching + explicit raise in non-local envs. That duplicates what Pro's configuration.rb already does at boot (validate_renderer_password_for_production) and diverges from the documented pattern in pro/node-renderer.md and pro/installation.md. Revert to the simple ENV.fetch with a dev default. Pro handles the prod-enforcement itself — if RENDERER_PASSWORD is unset in a production-like env, Pro raises at boot with a helpful error message pointing at exactly this fix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Pin react and react-dom to 19.0.4 Journey plan's Sub-PR 1 KEEP table explicitly called for the 19.0.4 pin from PR #723's final state — missed in the Sub-PR 1 migration commit. 19.0.4 is the minimum React version documented as required for React Server Components (per the Pro RSC tutorial doc, CVE-safe floor). Tightens from the inherited "^19.0.0" range. Boot-verified: webpack build clean; renderer + Rails boot; GET / returns HTTP 200 with "Node Renderer responded" in Rails log and "[SERVER] RENDERED" in renderer log — SSR path unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Rename Procfile.dev renderer process to node-renderer Match the Pro dummy's Procfile.dev and PR #723's naming. Marketplace uses `renderer:`, but the dummy is the canonical Pro reference and aligns with how the :pro generator's `add_pro_to_procfile` names it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Default renderer logLevel to 'info' Pro docs (docs/oss/building-features/node-renderer/container-deployment.md) prescribe `logLevel: 'info'` as the general-renderer-logs default, and the Pro dummy's renderer/node-renderer.js also uses 'info'. The inherited 'debug' default is too verbose for normal operation (emits the full VM context setup messages like "Adding Buffer, TextDecoder, ..."). Debug remains reachable via RENDERER_LOG_LEVEL=debug env var as docs describe for active debugging (docs/oss/building-features/node-renderer/ debugging.md). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Trim over-verbose comments across Sub-PR 2 files Per the repo's CLAUDE.md guidance ("default to writing no comments; only when the WHY is non-obvious"), stripping comments that explain WHAT the code does or reference the current-PR context. Kept (non-obvious WHYs): the `additionalContext: { URL, AbortController }` reason (react-router-dom NavLink url-is-not-defined), the CI worker-count cap reason, `clientReferences` scoping rationale, `libraryTarget: 'commonjs2'` requirement for the renderer, `target: 'node'` fix for SSR-breaking libs, the renderer-fallback-disabled reason, the renderer-password-must-match pointer, the bundler-utils dual-support blurb, and the shakapacker.yml tactical-reversal tag. Dropped (WHAT explanations, redundant with identifiers): JSDoc blocks describing `configureServer` and `extractLoader`, per-config-key descriptions in the renderer launcher, Procfile.dev process-purpose comment, verbose `.env.example` prose, initializer multi-line explanations of each field. No code changes — comments only. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Restore license setup guidance in initializer comments Over-trimmed in the previous cleanup commit. The longer wording has non-obvious info (where to get the JWT, which envs skip license, what Pro does when no license is set) that readers genuinely benefit from — beyond what the generator template's one-line version captures. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Wire the Pro Node renderer for Control Plane deploys Sub-PR 2 already wires dev (Procfile.dev) and CI (rspec_test.yml). Without this commit, merging base → master with the renderer active on the Rails side but no renderer process in the deployed container would break the live tutorial (Rails tries to hit localhost:3800, nothing listens, HTTPX::ConnectionError → 500 with renderer_use_fallback_exec_js = false). Matches the react-server-components-marketplace-demo deploy pattern (Option 1 "Single Container" from docs/oss/building-features/ node-renderer/container-deployment.md): - .controlplane/Dockerfile: CMD starts the Node renderer and Rails in the same container with `wait -n ; exit 1` so any child exit restarts the whole workload. Preserves the existing Thruster HTTP/2 proxy for Rails — the only deviation from marketplace's literal CMD. - .controlplane/templates/app.yml: add RENDERER_PORT, RENDERER_LOG_LEVEL, RENDERER_WORKERS_COUNT, RENDERER_URL as plain env, plus RENDERER_PASSWORD and REACT_ON_RAILS_PRO_LICENSE as Control Plane secret references keyed by {{APP_NAME}}-secrets (matches the repo's existing {{APP_NAME}} templating convention in DATABASE_URL / REDIS_URL). - .controlplane/templates/rails.yml: bump memory 512Mi → 2Gi. The container now runs two processes; 512Mi OOMs fast. 2Gi matches the marketplace demo's rails.yml. - config/initializers/react_on_rails_pro.rb: read ENV["RENDERER_URL"] instead of ENV["REACT_RENDERER_URL"]. Aligns with docs/oss/ configuration/configuration-pro.md (which uses RENDERER_URL), the Pro dummy's initializer, and the marketplace initializer — all of which use RENDERER_URL without a REACT_ prefix. The REACT_ prefix was inherited from PR #723 and is non-canonical. - .env.example: matching REACT_RENDERER_URL → RENDERER_URL rename. Prerequisite before first deploy (outside PR scope, one-time per app per org, performed manually via cpln or the Control Plane UI): create a Secret named `<app-name>-secrets` with keys RENDERER_PASSWORD (strong random) and REACT_ON_RAILS_PRO_LICENSE (JWT from pro.reactonrails.com). Affects react-webpack-rails-tutorial-production, react-webpack-rails-tutorial-staging, and any qa-* review apps. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Use {{APP_SECRETS}} for renderer + license secret references {{APP_NAME}}-secrets expanded to a per-app secret name, which would require a new Control Plane Secret for every review app PR — wrong per cpflow's own conventions. cpflow exposes {{APP_SECRETS}} (lib/core/template_parser.rb:49, lib/core/config.rb:51-52) which expands to `{APP_PREFIX}-secrets`. Per our controlplane.yml, APP_PREFIX is: - `react-webpack-rails-tutorial-production` for the prod app - `react-webpack-rails-tutorial-staging` for the staging app - `qa-react-webpack-rails-tutorial` for all qa-* review apps (because match_if_app_name_starts_with: true) So review apps all share `qa-react-webpack-rails-tutorial-secrets` instead of each PR needing its own. Three secrets total across two orgs instead of one per PR. Matches the `{APP_PREFIX}-secrets` default documented at shakacode/control-plane-flow/docs/secrets-and-env-values.md. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Fix env var name mismatch: NODE_RENDERER_CONCURRENCY → RENDERER_WORKERS_COUNT The launcher was reading NODE_RENDERER_CONCURRENCY (inherited from PR #723's f55dcc2), but app.yml sets RENDERER_WORKERS_COUNT (canonical per marketplace + Pro renderer library). Result: prod/staging would ignore the deployed value and always use the launcher default. RENDERER_WORKERS_COUNT is the canonical env var name per: - docs/oss/building-features/node-renderer/js-configuration.md "workersCount (default: process.env.RENDERER_WORKERS_COUNT ...)" - packages/react-on-rails-pro-node-renderer/src/shared/configBuilder.ts:200 "workersCount: env.RENDERER_WORKERS_COUNT ? parseInt(...) : defaultWorkersCount()" - react-server-components-marketplace-demo/node-renderer.js NODE_RENDERER_CONCURRENCY was a non-canonical name invented by PR #723. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Right-size rails workload: 1Gi + capacityAI Previous commit bumped memory 512Mi → 2Gi matching the marketplace demo, but cherry-picked only the memory dimension: CPU stayed at 300m and capacityAI stayed false. Net result was a wasteful fixed 2Gi allocation with no autoscale — the pattern Justin flagged. Better right-size for this tutorial (smaller Rails surface than the marketplace RSC demo): - memory: 2Gi → 1Gi. Enough headroom for Rails + the Pro Node renderer in one container without reserving capacity that won't be used. - capacityAI: false → true. Adjusts CPU/memory within the single replica based on observed usage, so the 1Gi/300m baseline grows if actual workload warrants it. Matches the marketplace demo's capacityAI posture without copying its oversized static baseline. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Replace org.yml placeholder with renderer + license schema Document the actual keys the qa-* dictionary needs (RENDERER_PASSWORD, REACT_ON_RAILS_PRO_LICENSE) instead of the unused SOME_ENV placeholder, and warn against re-applying the template after real values are populated. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Fix stale Rspack comments in Procfile.dev The rspack→webpack bundler flip in this PR left two comment lines referring to Rspack that now misdescribe the active bundler. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Treat blank RENDERER_PASSWORD as unset in Rails initializer ENV.fetch returns "" when the env var is set to empty string, while the JS renderer's `process.env.RENDERER_PASSWORD || fallback` treats "" as falsy. Result: copying .env.example verbatim (ships with a blank RENDERER_PASSWORD= line) leaves Rails sending "" in the auth header while the renderer expects the dev default, silently failing SSR auth. Switch to ENV["RENDERER_PASSWORD"].presence || default so blank values route to the fallback on both sides. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Document RENDERER_PORT, LOG_LEVEL, WORKERS_COUNT in .env.example app.yml and the renderer launcher read three additional env vars that .env.example didn't mention, leaving developers with no single place to see which renderer knobs exist. Add them commented-out with their defaults and brief descriptions. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Derive RSCWebpackPlugin clientReferences from config.source_path Previously hardcoded path.resolve(__dirname, '../../client/app'), which would silently point at the wrong directory if shakapacker.yml's source_path were ever changed. The same file already derives the server-bundle entry's path from config.source_path (line 33); apply the same pattern here so the two stay in sync. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Dump Node renderer log on CI rspec failure The renderer startup step only cats /tmp/node-renderer.log when the startup wait loop times out (30s). If the renderer starts fine but crashes mid-test, the log is never surfaced, making the rspec failure impossible to debug from the Actions UI. Add an `if: failure()` step after rspec that cats the log so any crash during the test run is visible. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Forward SIGTERM to children in Dockerfile CMD Bash as PID 1 doesn't forward signals to its children by default, so Control Plane's rolling-deploy SIGTERM never reaches Puma or the Node renderer's cluster manager. Both handle SIGTERM automatically (drain in-flight requests, stop accepting new ones), but only if they receive the signal. Without a trap, the graceful period expires and SIGKILL drops in-flight requests on every rolling deploy. Add `trap 'kill -TERM $RENDERER_PID $RAILS_PID' TERM INT` before `wait -n`. Uses the PID captures that were previously assigned but unused, turning dead code into a real graceful-shutdown mechanism aligned with the graceful-shutdown section of container-deployment.md. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Revert "Forward SIGTERM to children in Dockerfile CMD" This reverts commit 1bd5e86. The trap addition was incomplete. On the shutdown signal (Control Plane sends SIGINT per docs.controlplane.com/reference/workload/termination.md), the trap fires, `wait -n` returns 130, and `exit 1` then runs unconditionally. The container exits with code 1 on every rolling deploy instead of a code that reflects the signal-initiated shutdown. More importantly, the trap was trying to solve a problem Control Plane already handles. CP's default preStop hook runs `sh -c "sleep 45"` before any signal reaches PID 1, and the sidecar stops accepting new inbound connections ~80 seconds ahead of termination. That sleep plus the routing drain is the graceful-shutdown window; signal forwarding inside the container is marginal on top of it. Match the reference deployments verbatim: the react-on-rails-demo- marketplace-rsc Dockerfile and the hichee production Dockerfile both use this exact bash-c pattern without a trap. If deeper graceful shutdown is ever needed, the right tool is extending preStop or switching to a process manager (overmind, tini), not a bash trap on wait -n. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Guard against dev-default RENDERER_PASSWORD in production The Pro renderer's JS side raises if RENDERER_PASSWORD is unset in production, but accepts the literal "local-dev-renderer-password" value. A CP secret misconfigured to that string (easy to happen when copying from .env.example) would let both sides "match" while running with no real authentication. Split the branch by Rails.env.local?: - dev/test keeps the .presence || default fallback so blank env vars still work for local development - production fetches strict and raises if blank, unset, or the literal dev default Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Fold CI workers cap into parseIntegerEnv default The post-hoc `if (process.env.CI && env == null) config.workersCount = 2` block mutated an already-constructed config and used a narrower definition of "unset" (`== null`) than parseIntegerEnv's own check (treats "" as unset too). Folding the CI default into the second argument of parseIntegerEnv: workersCount: parseIntegerEnv('RENDERER_WORKERS_COUNT', process.env.CI ? 2 : 3, { min: 0 }) keeps the same behaviour for explicit values, uses one definition of "unset" consistently, and drops the mutation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Document rscBundle parameter's Sub-PR 3 role The parameter is unused until Sub-PR 3 wires rscWebpackConfig.js to call configureServer(true). Adding a short comment so it isn't mistaken for dead code during review. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Revert "Document rscBundle parameter's Sub-PR 3 role" This reverts commit a7734472b8e9e2bf80088a85ef0b7b9dd76b44a0. The `rscBundle = false` parameter shape follows the documented Pro pattern: docs/oss/migrating/rsc-preparing-app.md:244 and docs/pro/react-server-components/upgrading-existing-pro-app.md:106 both name it by parameter name and show the exact `if (!rscBundle) { ... RSCWebpackPlugin ... }` guard. The Pro dummy (react_on_rails_pro/spec/dummy/config/webpack/serverWebpackConfig.js) uses the same shape with no inline comment. The reverted commit restated what the docs already cover and added a Sub-PR 3 reference that would rot once Sub-PR 3 merges. Code follows documented conventions; readers who need context can read the doc. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Loosen react pin from exact 19.0.4 to ~19.0.4 The exact pin locked out every 19.0.x patch including 19.0.5 and future security patches within the 19.0 line. Pro's own install docs (docs/pro/react-server-components/upgrading-existing-pro-app.md:26-28 and create-without-ssr.md:37) prescribe `react@~19.0.4` — tilde range that keeps the CVE floor while allowing 19.0.x patches. react-on-rails-rsc@19.0.4's peer dep is `react: ^19.0.3`, so 19.0.5 satisfies it. After this change, yarn resolves react to 19.0.5. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Trim renderer_password initializer comment The previous comment restated in English what the code below already expressed. Cut to the one non-obvious WHY (why the prod branch exists despite Pro's own JS-side guard). Method names + the raise message cover the rest. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Align CI renderer log handling with Pro CI Pro's own integration CI (react_on_rails_pro/.github/workflows/ pro-integration-tests.yml) runs the renderer with `pnpm run node-renderer &` — no log redirect — so renderer output interleaves with job stdout and is always visible without a special dump step. Our workflow inherited a redirect-to-/tmp/node-renderer.log pattern from PR #723 plus a follow-up if-failure cat step (commit 7dea094) that dumped the file after rspec. Both existed to work around the redirect; neither was in any reference. Drop the redirect and the associated cat calls (startup-failure cat, timeout cat, post-rspec failure dump). Renderer logs now appear inline with job output, same shape as Pro CI. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Reframe shakapacker bundler note as TODO "Tactical:" read as a weird label; "TODO:" is the standard signal that the current state is meant to be reverted once the upstream blocker (shakacode/react_on_rails_rsc#29) ships. Same content, leads with the action. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Remove RENDERER_PASSWORD prod-default guard The guard added in 0eb94af raised at every Rails boot in production, which breaks the Docker build step that runs `bin/rails react_on_rails:locale` during image bake (RENDERER_PASSWORD isn't in the build environment — secrets are mounted at runtime only, not at build time). Deploy on Control Plane fails before it can even produce an image. Reference check on the guard: zero of Pro dummy, marketplace demo (react-on-rails-demo-marketplace-rsc), Justin's PR 723, and hichee have a "raise on literal dev-default value" guard. Each uses ENV.fetch with a default or hardcodes the password. The guard was a reviewer-driven divergence with no reference backing. Revert the prod branch. Keep the .presence fallback from 7254b1a (that one responds to our specific .env.example shape, not a reference divergence). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 participant
Summary
react_on_railsgem to16.4.0andshakapackergem to9.7.0react-on-railsto16.4.0andshakapackerto9.7.0@rspack/core/@rspack/cli=2.0.0-beta.7) and enforce rspack-only bundler pathNotes
@rspack/core2.0.0-rc.*; this uses the latest published v2 prerelease (2.0.0-beta.7).yarn build:devis blocked locally unless Ruby3.4.6is available (Gemfile requires 3.4.6).Validation
yarn jest client/__tests__/webpack/bundlerUtils.spec.jsyarn jest client/__tests__/swc-config.spec.jsxSummary by CodeRabbit
New Features
Dependencies
Documentation
Tests
Chores / Breaking Changes
Note
Medium Risk
Build pipeline and dependency upgrades (including
@rspack/*v2 beta) can change asset compilation/HMR behavior and may break local/dev or CI builds if assumptions differ. Runtime/business logic is largely untouched, but SSR/client bundling paths are now stricter (Rspack-only) and will fail fast on misconfig.Overview
Upgrades the React-on-Rails stack to stable
react_on_rails/react-on-rails16.4.0andshakapacker9.7.0, and moves the JS bundler to@rspack/core/@rspack/cli2.0.0-beta.7(plus@rspack/plugin-react-refresh).Standardizes the build configuration on Rspack-only:
bundlerUtilsnow throws unlessassets_bundler: rspack, client/server webpack configs load the bundler exclusively viagetBundler(), and dev react-refresh wiring is delegated to Shakapacker’s Rspack dev-server integration.Updates developer workflows and docs to be Rspack-first (Procfiles, README copy/links), bumps ControlPlane Docker Node version to
22.12.0, adjusts.gitignoreto commit.claude/commands, and adds a new.claude/commands/address-review.mdhelper command for PR review triage/replies.Written by Cursor Bugbot for commit e23c883. This will update automatically on new commits. Configure here.