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 👍 / 👎.
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.