Improve create/install validation flows and migration docs#2650
Improve create/install validation flows and migration docs#2650
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughGenerator, CLI, and docs updated to add multi-bundler (Rspack/Webpack) support, pnpm normalization and package-manager override, generator-driven TypeScript-first installs, bin/dev replacement detection, added bin/rails db:prepare to success messaging, expanded smoke-tests, and many supporting template, system-checker, git-utils, and test changes. (49 words) Changes
Sequence DiagramsequenceDiagram
participant User
participant CLI as create-react-on-rails-app
participant Generator as Rails Generator
participant PM as Package Manager
participant App as Generated App
User->>CLI: run create command (flags: --typescript / --rspack / --rsc)
CLI->>Generator: run react_on_rails:install (env REACT_ON_RAILS_PACKAGE_MANAGER)
Generator->>Generator: detect bundler & template (webpack vs rspack, ts vs js)
Generator->>PM: install react_on_rails & JS deps (uses chosen PM)
PM->>App: write lockfiles / node_modules
CLI->>App: normalize package manager (pnpm conversion if detected)
App->>App: update package.json & bin/setup, remove package-lock.json
CLI->>Generator: copy bin scripts (replace stock bin/dev when detected)
CLI->>User: print success (includes `bin/rails db:prepare` and docs link)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
size-limit report 📦
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
Review Summary Good overall direction. The UX improvements (warn instead of hard-fail on dirty worktrees, automatic bin/dev replacement, pnpm normalization) are solid additions, and the new vite_rails migration guide fills a real gap. A few issues worth addressing before merge. Bugs
Code clarity
Docs
Minor / non-blocking
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Stale comment contradicts new non-blocking behavior
- Updated the installation prerequisite comment to accurately describe that dirty-tree state now triggers a warning before potential Pro Gemfile auto-install mutation.
Or push these changes by commenting:
@cursor push 42e1e334aa
Preview (42e1e334aa)
diff --git a/react_on_rails/lib/generators/react_on_rails/install_generator.rb b/react_on_rails/lib/generators/react_on_rails/install_generator.rb
--- a/react_on_rails/lib/generators/react_on_rails/install_generator.rb
+++ b/react_on_rails/lib/generators/react_on_rails/install_generator.rb
@@ -192,8 +192,8 @@
# js(.coffee) are not checked by this method, but instead produce warning messages
# and allow the build to continue
def installation_prerequisites_met?
- # Check uncommitted_changes? before missing_pro_gem? so that
- # auto-install does not mutate the Gemfile on a dirty working tree.
+ # Warn before checking missing_pro_gem? so users are notified before
+ # any potential Gemfile mutation from auto-install.
ReactOnRails::GitUtils.warn_if_uncommitted_changes(GeneratorMessages)
!(missing_node? || missing_package_manager? || missing_pro_gem?)
Greptile SummaryThis PR improves the developer experience for both fresh app creation and existing-app installation by reducing interactive friction and providing better guidance.
Confidence Score: 4/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["npx create-react-on-rails-app my-app --package-manager pnpm"] --> B["rails new my-app --database=postgresql --skip-javascript"]
B --> C["bundle add react_on_rails --strict"]
C --> D{"--rsc flag?"}
D -->|Yes| E["bundle add react_on_rails_pro"]
D -->|No| F["Run generator"]
E --> F
F --> G["bundle exec rails generate react_on_rails:install --force --ignore-warnings\n(env: REACT_ON_RAILS_PACKAGE_MANAGER=pnpm)"]
G --> H{"Package manager\n== pnpm?"}
H -->|Yes| I["normalizeGeneratedPackageManager"]
H -->|No| K["Print success message"]
I --> I1["Set packageManager field in package.json"]
I1 --> I2["pnpm import (convert npm lockfile)"]
I2 --> I3["Remove package-lock.json"]
I3 --> I4["Rewrite bin/setup: npm → pnpm"]
I4 --> I5["pnpm install"]
I5 --> K
subgraph "Generator (install_generator.rb)"
G --> G1["warn_if_uncommitted_changes\n(warning only, non-blocking)"]
G1 --> G2["Check prerequisites\n(node, package manager, pro gem)"]
G2 --> G3["replace_stock_rails_bin_dev!\n(auto-replace if stock Rails 8 bin/dev)"]
G3 --> G4["invoke react_on_rails:base"]
end
Last reviewed commit: 79c4fad |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
packages/create-react-on-rails-app/scripts/smoke-test-local-gems.sh (1)
109-115: Consider adding route verification for the Rspack app.The TypeScript app (line 93) and JavaScript app (line 102) both verify
hello_worldroute inroutes.rb, but this check is missing for the Rspack app. Since Rspack is a non-RSC app, it should also have thehello_worldroute.Proposed addition
grep -q "gem \"react_on_rails\"" "$APP_RSPACK_DIR/Gemfile" grep -q "path: \"$RUBY_GEM_DIR\"" "$APP_RSPACK_DIR/Gemfile" +grep -q "hello_world" "$APP_RSPACK_DIR/config/routes.rb" grep -q '"@rspack/core"' "$APP_RSPACK_DIR/package.json"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/create-react-on-rails-app/scripts/smoke-test-local-gems.sh` around lines 109 - 115, Add a check for the hello_world route to the Rspack app smoke tests: in scripts/smoke-test-local-gems.sh locate the Rspack verification block that uses $APP_RSPACK_DIR and add a grep asserting that routes.rb contains the hello_world route (same pattern used for the TypeScript and JavaScript app checks). Ensure the new assertion uses grep -q against "$APP_RSPACK_DIR/config/routes.rb" (or the same routes path used elsewhere) and follows the existing test error handling conventions in the script.react_on_rails/lib/generators/react_on_rails/install_generator.rb (1)
323-323: Normalize line endings before stockbin/devcomparison.Line 323 can fail to detect stock content on CRLF checkouts. Normalizing
\r\nto\nmakes detection cross-platform.Suggested patch
- File.read("bin/dev").strip == STOCK_RAILS_BIN_DEV.strip + File.read("bin/dev").gsub("\r\n", "\n").strip == STOCK_RAILS_BIN_DEV.strip🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@react_on_rails/lib/generators/react_on_rails/install_generator.rb` at line 323, The equality check comparing File.read("bin/dev").strip to STOCK_RAILS_BIN_DEV.strip can fail on CRLF checkouts; normalize line endings before comparing by converting CRLF to LF for both sides (e.g., apply a .gsub or .delete on "\r" to the File.read result and to STOCK_RAILS_BIN_DEV) so the comparison in install_generator (the expression using File.read("bin/dev") and STOCK_RAILS_BIN_DEV) is platform independent.docs/oss/getting-started/installation-into-an-existing-rails-app.md (1)
29-31: Consider showing package-manager-agnostic install commands.The optional version pinning example only shows pnpm. Per repository guidelines, end-user documentation should include npm, yarn, and pnpm alternatives.
Suggested alternative
```bash -pnpm add [email protected] --save-exact +npm install [email protected] --save-exact +# or: yarn add [email protected] --exact +# or: pnpm add [email protected] --save-exact</details> Based on learnings: "In all end-user documentation under docs/, ensure package-manager-agnostic installation instructions include npm, yarn, and pnpm." <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@docs/oss/getting-started/installation-into-an-existing-rails-app.mdaround
lines 29 - 31, Replace the single pnpm install example with
package-manager-agnostic alternatives: update the command that installs
[email protected] so it shows npm, yarn, and pnpm variants (e.g., "npm
install [email protected] --save-exact", "yarn add
[email protected] --exact", and the existing "pnpm add
[email protected] --save-exact"); ensure the exact/version pinning
flag is correct for each package manager and replace the single-line snippet in
the installation example accordingly.</details> </blockquote></details> <details> <summary>docs/oss/upgrading/upgrading-react-on-rails.md (1)</summary><blockquote> `131-137`: **Consider showing package-manager-agnostic install commands.** Line 136 shows only `pnpm install`. Per repository guidelines, end-user documentation should include npm, yarn, and pnpm alternatives to accommodate users' different package manager choices. <details> <summary>Suggested alternative</summary> ```diff ```bash bundle update react_on_rails shakapacker # then run your package manager's install command - pnpm install + npm install # or yarn install / pnpm install ```Based on learnings: "In all end-user documentation under docs/, ensure package-manager-agnostic installation instructions include npm, yarn, and pnpm."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/oss/upgrading/upgrading-react-on-rails.md` around lines 131 - 137, The "Install Updates" step currently shows only the literal command "pnpm install"; update this to include package-manager-agnostic alternatives by replacing or augmenting the single "pnpm install" line with a combined suggestion such as "npm install # or yarn install / pnpm install" so readers using npm, yarn, or pnpm are all covered; locate the "Install Updates" section and the line containing "pnpm install" and modify that line accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@react_on_rails/lib/generators/react_on_rails/install_generator.rb`:
- Around line 197-199: The call to
ReactOnRails::GitUtils.warn_if_uncommitted_changes is using the default
git_installed: true and therefore never reports missing Git; update the call to
pass the actual Git availability by calling cli_exists?("git") as the
git_installed argument. Locate the call to
ReactOnRails::GitUtils.warn_if_uncommitted_changes (near the end of
install_generator.rb) and change it to pass cli_exists?("git"), while leaving
the final returned expression !(missing_node? || missing_package_manager? ||
missing_pro_gem?) unchanged.
---
Nitpick comments:
In `@docs/oss/getting-started/installation-into-an-existing-rails-app.md`:
- Around line 29-31: Replace the single pnpm install example with
package-manager-agnostic alternatives: update the command that installs
[email protected] so it shows npm, yarn, and pnpm variants (e.g., "npm
install [email protected] --save-exact", "yarn add
[email protected] --exact", and the existing "pnpm add
[email protected] --save-exact"); ensure the exact/version pinning
flag is correct for each package manager and replace the single-line snippet in
the installation example accordingly.
In `@docs/oss/upgrading/upgrading-react-on-rails.md`:
- Around line 131-137: The "Install Updates" step currently shows only the
literal command "pnpm install"; update this to include package-manager-agnostic
alternatives by replacing or augmenting the single "pnpm install" line with a
combined suggestion such as "npm install # or yarn install / pnpm install" so
readers using npm, yarn, or pnpm are all covered; locate the "Install Updates"
section and the line containing "pnpm install" and modify that line accordingly.
In `@packages/create-react-on-rails-app/scripts/smoke-test-local-gems.sh`:
- Around line 109-115: Add a check for the hello_world route to the Rspack app
smoke tests: in scripts/smoke-test-local-gems.sh locate the Rspack verification
block that uses $APP_RSPACK_DIR and add a grep asserting that routes.rb contains
the hello_world route (same pattern used for the TypeScript and JavaScript app
checks). Ensure the new assertion uses grep -q against
"$APP_RSPACK_DIR/config/routes.rb" (or the same routes path used elsewhere) and
follows the existing test error handling conventions in the script.
In `@react_on_rails/lib/generators/react_on_rails/install_generator.rb`:
- Line 323: The equality check comparing File.read("bin/dev").strip to
STOCK_RAILS_BIN_DEV.strip can fail on CRLF checkouts; normalize line endings
before comparing by converting CRLF to LF for both sides (e.g., apply a .gsub or
.delete on "\r" to the File.read result and to STOCK_RAILS_BIN_DEV) so the
comparison in install_generator (the expression using File.read("bin/dev") and
STOCK_RAILS_BIN_DEV) is platform independent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4da2ea48-2770-4e2a-a2ec-868ad8108982
📒 Files selected for processing (16)
README.mddocs/oss/getting-started/create-react-on-rails-app.mddocs/oss/getting-started/installation-into-an-existing-rails-app.mddocs/oss/migrating/migrating-from-react-rails.mddocs/oss/migrating/migrating-from-vite-rails.mddocs/oss/upgrading/upgrading-react-on-rails.mdpackages/create-react-on-rails-app/scripts/smoke-test-local-gems.shpackages/create-react-on-rails-app/src/create-app.tspackages/create-react-on-rails-app/src/index.tspackages/create-react-on-rails-app/src/utils.tspackages/create-react-on-rails-app/tests/create-app.test.tsreact_on_rails/lib/generators/react_on_rails/generator_messages.rbreact_on_rails/lib/generators/react_on_rails/install_generator.rbreact_on_rails/lib/react_on_rails/git_utils.rbreact_on_rails/spec/react_on_rails/generators/install_generator_spec.rbreact_on_rails/spec/react_on_rails/git_utils_spec.rb
|
Addressed the review items in 5d699ef. Changes: pass actual git availability into the dirty-worktree warning, normalize CRLF when detecting stock |
PR Review: Improve create/install validation flows and migration docsOverall this is a solid improvement to the create/install UX. The warning-instead-of-hard-fail for dirty worktrees, pnpm normalization, and --force passthrough are all welcome. A few issues worth addressing: BUGS / CORRECTNESS:
SECURITY:
DESIGN / MAINTAINABILITY:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
docs/oss/getting-started/installation-into-an-existing-rails-app.md (1)
23-23: Tighten wording on Line 23 (“same exact”).Consider changing “same exact release” to “same release” for cleaner phrasing.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/oss/getting-started/installation-into-an-existing-rails-app.md` at line 23, Replace the phrase "same exact release" with "same release" in the sentence starting "If you manage versions manually, keep the Ruby gem and npm package on the same exact release." so the wording reads more cleanly; update the text in the docs/getting-started sentence containing "same exact release" to "same release" while preserving the rest of the sentence and the note about pre-release gem/npm separator differences.docs/oss/upgrading/upgrading-react-on-rails.md (1)
29-29: Optional: Minor style suggestion.Static analysis suggests considering an alternative to "very old" for variety, but the current phrasing is clear and appropriate in this context. This is purely a stylistic nitpick.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/oss/upgrading/upgrading-react-on-rails.md` at line 29, Replace the phrase "very old apps" in the sentence "very old apps may have lockfiles created by Bundler 1.x" with a slightly more formal alternative (e.g., "significantly dated apps", "legacy apps", or "much older apps") to vary tone while keeping meaning; update the line in the upgrading-react-on-rails.md content so the sentence reads smoothly with the chosen replacement and preserves the rest of the wording about Bundler 1.x lockfiles failing on modern Ruby.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/create-react-on-rails-app/scripts/smoke-test-local-gems.sh`:
- Around line 109-116: Add a smoke-test assertion that the generated HelloWorld
component exists for the non-RSC Rspack app; update
scripts/smoke-test-local-gems.sh to check for the HelloWorld component file
(e.g., test -f "$APP_RSPACK_DIR/app/javascript/components/HelloWorld.jsx" ||
test -f "$APP_RSPACK_DIR/app/javascript/components/HelloWorld.tsx" or a grep -q
"HelloWorld" against that components directory) so the script validates template
output similar to the TS app check.
---
Nitpick comments:
In `@docs/oss/getting-started/installation-into-an-existing-rails-app.md`:
- Line 23: Replace the phrase "same exact release" with "same release" in the
sentence starting "If you manage versions manually, keep the Ruby gem and npm
package on the same exact release." so the wording reads more cleanly; update
the text in the docs/getting-started sentence containing "same exact release" to
"same release" while preserving the rest of the sentence and the note about
pre-release gem/npm separator differences.
In `@docs/oss/upgrading/upgrading-react-on-rails.md`:
- Line 29: Replace the phrase "very old apps" in the sentence "very old apps may
have lockfiles created by Bundler 1.x" with a slightly more formal alternative
(e.g., "significantly dated apps", "legacy apps", or "much older apps") to vary
tone while keeping meaning; update the line in the upgrading-react-on-rails.md
content so the sentence reads smoothly with the chosen replacement and preserves
the rest of the wording about Bundler 1.x lockfiles failing on modern Ruby.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 42341603-75a2-4bf7-b4b9-bdea6860830b
📒 Files selected for processing (4)
docs/oss/getting-started/installation-into-an-existing-rails-app.mddocs/oss/upgrading/upgrading-react-on-rails.mdpackages/create-react-on-rails-app/scripts/smoke-test-local-gems.shreact_on_rails/lib/generators/react_on_rails/install_generator.rb
🚧 Files skipped from review as they are similar to previous changes (1)
- react_on_rails/lib/generators/react_on_rails/install_generator.rb
|
Follow-up on RBS type signatures (Bug #1 from the review above): The file |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/create-react-on-rails-app/src/create-app.ts (1)
89-100: Consider adding error handling for malformed JSON.If
package.jsonexists but contains invalid JSON,JSON.parsewill throw an unhandled exception. While this is unlikely for generator-created files, a corrupted file could cause an unclear error.🛡️ Optional: wrap JSON parsing in try-catch
function updateJsonFile( filePath: string, updater: (data: Record<string, unknown>) => Record<string, unknown>, ): void { if (!fs.existsSync(filePath)) { return; } - const json = JSON.parse(fs.readFileSync(filePath, 'utf8')) as Record<string, unknown>; - const updatedJson = updater(json); - fs.writeFileSync(filePath, `${JSON.stringify(updatedJson, null, 2)}\n`, 'utf8'); + try { + const json = JSON.parse(fs.readFileSync(filePath, 'utf8')) as Record<string, unknown>; + const updatedJson = updater(json); + fs.writeFileSync(filePath, `${JSON.stringify(updatedJson, null, 2)}\n`, 'utf8'); + } catch (error) { + logError(`Failed to update ${filePath}: ${error instanceof Error ? error.message : 'unknown error'}`); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/create-react-on-rails-app/src/create-app.ts` around lines 89 - 100, The updateJsonFile function should handle malformed JSON by wrapping the file read/JSON.parse in a try-catch: in updateJsonFile, catch JSON parsing errors when reading filePath (the JSON.parse of fs.readFileSync) and either throw a new, descriptive error (including filePath and the original error) or log and rethrow so callers get a clear message instead of an unhandled exception; ensure the rest of the function still writes updatedJson only when parsing succeeds.docs/oss/getting-started/installation-into-an-existing-rails-app.md (1)
62-73: Helpful "What the generator changes" section.This transparency about generated files helps users understand what to review. One minor note: the server bundle path (
app/javascript/packs/server-bundle.js) depends on Shakapacker configuration and may differ in some setups.Consider softening the path reference:
- `config/shakapacker.yml` - `bin/dev` -- `app/javascript/packs/server-bundle.js` +- server bundle entry point (typically in `app/javascript/packs/`) - example `HelloWorld` component files🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/oss/getting-started/installation-into-an-existing-rails-app.md` around lines 62 - 73, The docs list a hard-coded server bundle path (`app/javascript/packs/server-bundle.js`) which can vary with Shakapacker config; change the bullet for the server bundle to a softened form that indicates this is a common default (e.g., "server bundle (commonly `app/javascript/packs/server-bundle.js`, path may vary based on `config/shakapacker.yml`)") so readers know to check `config/shakapacker.yml` rather than assuming the exact path.packages/create-react-on-rails-app/scripts/smoke-test-local-gems.sh (1)
91-99: Reduce repeated pnpm normalization assertions with a helper.The same four pnpm checks are repeated across multiple app variants, which is easy to drift over time. Consider extracting a small assertion helper and reusing it.
Refactor sketch
+assert_pnpm_normalized() { + local app_dir="$1" + test -f "$app_dir/pnpm-lock.yaml" + ! test -f "$app_dir/package-lock.json" + grep -q '"packageManager": "pnpm@' "$app_dir/package.json" + grep -q 'system!("pnpm install")' "$app_dir/bin/setup" +} + grep -q "gem \"react_on_rails\"" "$APP_TS_DIR/Gemfile" ... -test -f "$APP_TS_DIR/pnpm-lock.yaml" -! test -f "$APP_TS_DIR/package-lock.json" -grep -q '"packageManager": "pnpm@' "$APP_TS_DIR/package.json" -grep -q 'system!("pnpm install")' "$APP_TS_DIR/bin/setup" +assert_pnpm_normalized "$APP_TS_DIR" ... -test -f "$APP_JS_DIR/pnpm-lock.yaml" -! test -f "$APP_JS_DIR/package-lock.json" -grep -q '"packageManager": "pnpm@' "$APP_JS_DIR/package.json" -grep -q 'system!("pnpm install")' "$APP_JS_DIR/bin/setup" +assert_pnpm_normalized "$APP_JS_DIR"Also applies to: 104-108, 126-130, 135-138, 144-147
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/create-react-on-rails-app/scripts/smoke-test-local-gems.sh` around lines 91 - 99, Extract the four repeated pnpm assertions into a helper shell function (e.g., assert_pnpm_normalized) in the smoke-test-local-gems.sh script that performs the four checks: test -f "$APP_TS_DIR/pnpm-lock.yaml", ! test -f "$APP_TS_DIR/package-lock.json", grep -q '"packageManager": "pnpm@' "$APP_TS_DIR/package.json", and grep -q 'system!("pnpm install")' "$APP_TS_DIR/bin/setup"; then replace each repeated block of those four lines (the occurrences around the current block and the other mentioned blocks) with a single call to assert_pnpm_normalized so behavior and exit semantics are preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/create-react-on-rails-app/scripts/smoke-test-local-gems.sh`:
- Around line 140-147: Add the missing assertions that verify core RSC files and
view usage for the RSC+Rspack path: mirror the checks from the other RSC
variants by adding grep/tests that confirm the server component files and the
RSC view invocation exist (e.g., assert presence of the RSC server components
directory/file names and the view render call), alongside the existing checks
for "hello_server", "\"@rspack/core\"" and package manager lines so the
RSC+Rspack template output coverage matches the other variants.
---
Nitpick comments:
In `@docs/oss/getting-started/installation-into-an-existing-rails-app.md`:
- Around line 62-73: The docs list a hard-coded server bundle path
(`app/javascript/packs/server-bundle.js`) which can vary with Shakapacker
config; change the bullet for the server bundle to a softened form that
indicates this is a common default (e.g., "server bundle (commonly
`app/javascript/packs/server-bundle.js`, path may vary based on
`config/shakapacker.yml`)") so readers know to check `config/shakapacker.yml`
rather than assuming the exact path.
In `@packages/create-react-on-rails-app/scripts/smoke-test-local-gems.sh`:
- Around line 91-99: Extract the four repeated pnpm assertions into a helper
shell function (e.g., assert_pnpm_normalized) in the smoke-test-local-gems.sh
script that performs the four checks: test -f "$APP_TS_DIR/pnpm-lock.yaml", !
test -f "$APP_TS_DIR/package-lock.json", grep -q '"packageManager": "pnpm@'
"$APP_TS_DIR/package.json", and grep -q 'system!("pnpm install")'
"$APP_TS_DIR/bin/setup"; then replace each repeated block of those four lines
(the occurrences around the current block and the other mentioned blocks) with a
single call to assert_pnpm_normalized so behavior and exit semantics are
preserved.
In `@packages/create-react-on-rails-app/src/create-app.ts`:
- Around line 89-100: The updateJsonFile function should handle malformed JSON
by wrapping the file read/JSON.parse in a try-catch: in updateJsonFile, catch
JSON parsing errors when reading filePath (the JSON.parse of fs.readFileSync)
and either throw a new, descriptive error (including filePath and the original
error) or log and rethrow so callers get a clear message instead of an unhandled
exception; ensure the rest of the function still writes updatedJson only when
parsing succeeds.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8730785e-140f-462e-82cf-df562db2a995
📒 Files selected for processing (8)
docs/oss/getting-started/installation-into-an-existing-rails-app.mddocs/oss/upgrading/upgrading-react-on-rails.mdpackages/create-react-on-rails-app/scripts/smoke-test-local-gems.shpackages/create-react-on-rails-app/src/create-app.tspackages/create-react-on-rails-app/src/utils.tsreact_on_rails/lib/generators/react_on_rails/install_generator.rbreact_on_rails/lib/react_on_rails/git_utils.rbreact_on_rails/spec/react_on_rails/git_utils_spec.rb
🚧 Files skipped from review as they are similar to previous changes (2)
- react_on_rails/spec/react_on_rails/git_utils_spec.rb
- packages/create-react-on-rails-app/src/utils.ts
|
Code Review - see inline comments for specific locations. Overall this PR is a solid improvement. The pnpm normalization, warn-instead-of-fail git check, and expanded docs all move the install experience in the right direction. Bugs: COVERAGE=true incorrectly bypasses git checks (git_utils.rb line 58). ci_environment? treats COVERAGE=true as a CI signal, but SimpleCov and other Ruby coverage tools set this during local development. Only CI=true should gate the check. clean_worktree? swallows git errors (git_utils.rb line 63). Backticks only capture stdout. If git status exits non-zero, stdout is empty and the method silently returns true (clean). Use Open3.capture2e and check status.success?. UX / Docs: Generator success message omits bin/rails db:prepare. README and create-react-on-rails-app.md now include this step, but helpful_message_after_installation in generator_messages.rb does not. Users following the generator directly will miss it and hit a DB error on first bin/dev run. PostgreSQL is hardcoded with no escape hatch. Consider a --database option or a preflight check that postgres is reachable before rails new. Code quality: Brittle bin/setup regex for pnpm (create-app.ts line 139). The exact string pattern no-ops silently if Rails changes the template. Log a warning when no substitution occurs. --ignore-warnings suppresses all generator validation (create-app.ts line 78). If the intent is only to bypass the git-dirty check, a narrower flag would be safer. STOCK_RAILS_BIN_DEV has no version tracking (install_generator.rb lines 82-84). Silent misses for Rails 7.0 or future 9.x produce no feedback. Tracking the source template path and version aids maintenance. Tests:
|
- Fix stylesheet_pack_tag missing required argument in Vite migration doc
- Fix false "npm install" warning when bin/setup already uses pnpm
(JS substring match: "pnpm install".includes("npm install") is true)
- Restore COVERAGE=true env var guard in skip_worktree_check?
- Move rmSync after pnpm install so package-lock.json is preserved on failure
- Fix upgrade doc claiming Ruby 3.2+/Node 20+ when gemspec/package.json
require Ruby 3.0+/Node 18+
- Run pnpm install even when package-lock.json is absent (fallback path)
- Fix test that accidentally passed via wrong error path (JSON.parse failure
instead of pnpm import failure)
- Warn users when RSC route update is skipped due to custom bin/dev
Co-Authored-By: Claude Opus 4.6 <[email protected]>
- git_utils.rb: Deduplicate uncommitted_changes? and warn_if_uncommitted_changes into shared report_worktree_issues; use exit code 128 instead of fragile substring match for not-a-git-repo detection; return :git_not_installed for Errno::ENOENT instead of misreporting as :not_a_git_repository - install_generator.rb: Clarify non-blocking worktree warning intent; add explicit boolean coercion for preserve_existing_bin_dev? - create-app.ts: Fix misleading --force comment; broaden bin/setup npm regex to match system() and system!(); add null version warning for getCommandVersion; clarify PostgreSQL message - system_checker.rb: Document webpack default rationale when both configs exist - generator_messages.rb: Make REACT_ON_RAILS_PACKAGE_MANAGER env var case-insensitive with strip and downcase - comparison-with-alternatives.md: Fix bundler support to show Webpack/Rspack - create-app.test.ts: Simplify existsSync mock in pnpm failure test - git_utils_spec.rb: Add exitstatus to Process::Status mocks Co-Authored-By: Claude Opus 4.6 <[email protected]>
…worktree When preserve_existing_bin_dev? is true, Dir.glob on the template directory still included "dev", so File.chmod modified the user's custom bin/dev permissions — contradicting the preservation logic. Extract bin_scripts_to_chmod helper that excludes "dev" when preserving. Also restore the safety gate that prevents missing_pro_gem? from running (and potentially mutating Gemfile via bundle add) when the worktree is dirty. The warn-not-block design is preserved: dirty worktree only warns, but the Gemfile mutation is skipped to avoid mixing generated changes with uncommitted user work. Co-Authored-By: Claude Opus 4.6 <[email protected]>
…style bug - Add read-only pro_gem_installed? check when --pro/--rsc is used on a dirty worktree, so missing gem is caught without triggering auto-install that would mutate Gemfile - Fix Vite migration guide: use empty pack tags matching generator output, clarify process.env needs webpack DefinePlugin for client bundles - Fix quote style preservation in pnpm bin/setup regex replacement - Correct PostgreSQL prerequisite claim in CLI docs (not validated by CLI) Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
justin808
force-pushed
the
jg-codex/docs-validation-phase1
branch
from
83c65e5 to
725bcfd
Compare
| # Exit code 128 is git's standard fatal error (e.g., not a git repository) | ||
| return :not_a_git_repository if status.exitstatus == 128 | ||
|
|
||
| :dirty |
There was a problem hiding this comment.
Ambiguous fallthrough: non-128 git failures are treated as "dirty"
The current logic maps any git exit code that isn't 0 (clean) or 128 (not-a-repo) to :dirty. This means a transient git error — permission denied, stale lock file, disk I/O issue, etc. — would surface to the user as "You have uncommitted changes", which is misleading.
Git also uses exit code 1 for some advisory situations, but exit codes 2, 130 (SIGINT), and others indicate a real failure. Consider adding an explicit :error status for unrecognised non-zero/non-128 codes:
output, status = Open3.capture2e("git", "status", "--porcelain")
return :clean if status.success? && output.strip.empty?
return :not_a_git_repository if status.exitstatus == 128
return :dirty unless output.strip.empty?
:error # git ran but failed for an unrelated reasonThen report_worktree_issues can return false for :error (or log a distinct warning) so users aren't told their tree is dirty when git itself failed.
| if (fs.existsSync(packageLockPath)) { | ||
| execLiveArgs('pnpm', ['import'], appPath); | ||
| execLiveArgs('pnpm', ['install'], appPath); | ||
| fs.rmSync(packageLockPath, { force: true }); |
There was a problem hiding this comment.
Partial-failure state leaves misleading manual recovery steps
If pnpm import succeeds but pnpm install fails, the exception bubbles up to the outer catch block, which prints:
pnpm import
rm -f package-lock.json
pnpm install
But at this point pnpm-lock.yaml was already created by the successful pnpm import. Re-running pnpm import when a pnpm-lock.yaml already exists may either fail or produce unexpected results depending on the pnpm version.
The recovery instructions should check for the presence of pnpm-lock.yaml first, or at minimum note that the user should skip pnpm import if the file was already created:
if (fs.existsSync(packageLockPath)) {
execLiveArgs('pnpm', ['import'], appPath);
fs.rmSync(packageLockPath, { force: true });
}
execLiveArgs('pnpm', ['install'], appPath);Removing package-lock.json before running pnpm install also avoids the install seeing a conflicting lockfile.
| return false | ||
| end | ||
|
|
||
| !(missing_node? || missing_package_manager? || (!has_worktree_issues && missing_pro_gem?)) |
There was a problem hiding this comment.
Silent change in missing_pro_gem? evaluation on dirty worktrees
Previously, missing_pro_gem? was always evaluated (and could auto-install on any worktree state). Now it's only called when !has_worktree_issues. On a dirty worktree with a non-Pro install (use_pro? = false), the earlier guard block doesn't fire, and the short-circuit (!has_worktree_issues && missing_pro_gem?) = (false && ...) means missing_pro_gem? is never called at all.
This is almost certainly the right behaviour — there's no point running the auto-install check for non-Pro installs on a dirty tree — but a brief inline comment would make the intent explicit:
# On a dirty worktree, skip missing_pro_gem? to avoid auto-installing into an
# uncommitted Gemfile. Non-pro installs are always safe to skip this check.
!(missing_node? || missing_package_manager? || (!has_worktree_issues && missing_pro_gem?))
| ```erb | ||
| <%= stylesheet_pack_tag %> | ||
| <%= javascript_pack_tag %> |
There was a problem hiding this comment.
Empty javascript_pack_tag / stylesheet_pack_tag calls may not work as shown
Shakapacker's javascript_pack_tag and stylesheet_pack_tag helpers require a pack name argument in non-auto-bundling setups (they will raise Webpacker::Helper::AssetNotPrecompiledError or similar when the pack name is missing). For auto-bundling the helpers are typically omitted entirely from the layout (React on Rails injects the script tags itself via react_component).
Consider updating the snippet to one of:
- Show the correct auto-bundling layout (no explicit pack tags needed)
- Show the explicit pack name for a manual entrypoint:
javascript_pack_tag "application"
The clarifying sentence in the following paragraph is helpful, but readers who copy-paste the code block will likely hit an error.
|
|
||
| exec foreman start -f Procfile.dev $@ | ||
| SH | ||
| ].map { |template| template.gsub("\r\n", "\n").strip }.freeze |
There was a problem hiding this comment.
LEGACY_FOREMAN_BIN_DEV_TEMPLATES only covers 4 specific variants
This is a reasonable conservative approach (unknown variants are treated as custom), but it's worth noting that Rails 7.0 generated a slightly different bin/dev in some versions. There's also a potential for Rails to ship a new stock template in a patch release that wouldn't be recognised here, causing the generator to skip the replacement silently with a :skip status.
It might be worth adding a bin/dev template version comment (e.g. "last verified against Rails 7.2.x and 8.0.x") so future maintainers know when to re-check.
Review SummaryOverall this is a solid PR that meaningfully improves the create/install flow. The softened git checks, pnpm normalization, automatic bin/dev replacement, and expanded docs are all good additions. A few issues worth addressing before merge: Bug: worktree_status misreports git errors as dirty tree In git_utils.rb, the fallthrough returns :dirty for any git exit code that is not 0 or 128. A transient error (stale lock file, permission error, disk issue) will tell the user they have uncommitted changes when they do not. See the inline comment for a suggested fix. Bug: pnpm partial-failure leaves incorrect manual recovery steps In normalizeGeneratedPackageManager, if pnpm import succeeds but pnpm install fails, the recovery instructions tell the user to run pnpm import again - but pnpm-lock.yaml was already created by the successful first run. Re-importing over an existing pnpm lockfile can fail. The correct ordering should be: import, remove package-lock.json, then install. See the inline comment. Inconsistency: missing_node? still uses backtick shell execution cli_exists? was rightly updated to use system(which_command, command, ...) with no shell and proper Windows support. But missing_node? still uses backtick invocations and silently swallows non-zero exit codes. Not a security issue since the command is hardcoded, but creates an inconsistency that could bite on non-standard PATH configs. Doc: empty javascript_pack_tag / stylesheet_pack_tag example in vite migration guide The code block in migrating-from-vite-rails.md shows Shakapacker tag helpers with no arguments. Calling these with no pack name raises an error in non-auto-bundling setups. The explanatory prose is there, but readers who copy-paste the snippet will hit an error. Consider showing the explicit pack name or removing the code block for the auto-bundling case. Minor: LEGACY_FOREMAN_BIN_DEV_TEMPLATES coverage and maintainability The conservative approach (unrecognised templates are preserved) is correct. A comment noting the Rails versions these templates were verified against would help future maintainers know when to revisit. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 725bcfd10a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| 'bundle', | ||
| ['exec', 'rails', 'generate', 'react_on_rails:install', ...generatorArgs], | ||
| appPath, | ||
| { | ||
| ...process.env, | ||
| REACT_ON_RAILS_PACKAGE_MANAGER: options.packageManager, | ||
| }, | ||
| ); |
There was a problem hiding this comment.
If pnpm import or pnpm install fails after this bundle exec rails generate react_on_rails:install call, the Ruby generator has already streamed GeneratorMessages.helpful_message_after_installation (the “Successfully Installed” banner plus bin/dev/URL instructions) to stdout because this process runs with inherited stdio. The CLI exits non-zero later, but pnpm failures still look like successful scaffolds, which is misleading for both humans and automation consuming the command output.
Useful? React with 👍 / 👎.
## Summary - Add a dedicated evaluator guide comparing React on Rails with Hotwire/Turbo, Inertia Rails, Next.js + Rails API, and react-rails - Expand Inertia Rails comparison with performance tradeoffs (per-navigation round-trips, no code splitting/streaming SSR) and integration model differences (all-or-nothing page replacement vs incremental `react_component` adoption) - Add detailed feature matrix with build-tool benchmarks (Rspack vs Vite vs Webpack) - Route evaluators to the new guides from introduction, docs index, quick start, and tutorial ## Why Issue #2617 calls out the evaluator persona, but the docs still mostly force that user to infer tradeoffs from scattered pages. This PR gives evaluators one explicit decision page and links it from the main getting-started surfaces. ## What changed - Added `docs/oss/getting-started/comparing-react-on-rails-to-alternatives.md` — narrative comparison guide with short-version decision rules, per-framework sections, and common decision patterns - Added `docs/oss/getting-started/nextjs-with-separate-rails-backend.md` — dedicated Next.js + Rails API tradeoff guide - Updated `docs/oss/getting-started/comparison-with-alternatives.md` — expanded Inertia trade-offs section covering server round-trip cost, all-or-nothing adoption model, SSR limitations, controller coupling, and no RSC/caching path - Added evaluator-oriented links to `docs/README.md` and `docs/oss/introduction.md` - Updated `quick-start.md` to reflect the validated install command and added comparison guide as a next step ## Key content in the Inertia comparison - **Performance:** Every Inertia navigation requires a Rails round-trip with full page-props serialization; no code splitting with SSR; no streaming SSR - **Integration:** Inertia replaces Rails views per-route (all-or-nothing) vs React on Rails' `react_component` helper for incremental adoption in any ERB/Haml template - **Lock-in:** Inertia controllers are coupled to the Inertia protocol; React on Rails uses standard Rails rendering - **Feature gap:** No path to React Server Components or fragment caching ## Validation - `npm run prepare` in `reactonrails.com` - `npm run build` in `reactonrails.com` - `npm run audit:docs` in `reactonrails.com` - Spot-checked rendered pages in local browser ## Notes - This PR is stacked on top of #2650. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Low risk: documentation-only additions/edits with no runtime or API changes; primary risk is broken links or inaccurate guidance. > > **Overview** > Adds new evaluator-focused docs: a narrative decision guide comparing React on Rails with Hotwire/Turbo, Inertia Rails, Next.js+Rails API, react-rails, plus a dedicated page on the Next.js+separate Rails backend tradeoffs. > > Updates the existing alternatives matrix to expand the Inertia section with clearer adoption/performance limitations, and refreshes entry-point docs (`docs/README.md`, `oss/introduction.md`, `quick-start.md`) to link to the new guides and adjust the install instructions toward `--typescript` and `.tsx` examples. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 8ccad36. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Opus 4.6 <[email protected]>
## Summary - Add a dedicated evaluator guide comparing React on Rails with Hotwire/Turbo, Inertia Rails, Next.js + Rails API, and react-rails - Expand Inertia Rails comparison with performance tradeoffs (per-navigation round-trips, no code splitting/streaming SSR) and integration model differences (all-or-nothing page replacement vs incremental `react_component` adoption) - Add detailed feature matrix with build-tool benchmarks (Rspack vs Vite vs Webpack) - Route evaluators to the new guides from introduction, docs index, quick start, and tutorial ## Why Issue #2617 calls out the evaluator persona, but the docs still mostly force that user to infer tradeoffs from scattered pages. This PR gives evaluators one explicit decision page and links it from the main getting-started surfaces. ## What changed - Added `docs/oss/getting-started/comparing-react-on-rails-to-alternatives.md` — narrative comparison guide with short-version decision rules, per-framework sections, and common decision patterns - Added `docs/oss/getting-started/nextjs-with-separate-rails-backend.md` — dedicated Next.js + Rails API tradeoff guide - Updated `docs/oss/getting-started/comparison-with-alternatives.md` — expanded Inertia trade-offs section covering server round-trip cost, all-or-nothing adoption model, SSR limitations, controller coupling, and no RSC/caching path - Added evaluator-oriented links to `docs/README.md` and `docs/oss/introduction.md` - Updated `quick-start.md` to reflect the validated install command and added comparison guide as a next step ## Key content in the Inertia comparison - **Performance:** Every Inertia navigation requires a Rails round-trip with full page-props serialization; no code splitting with SSR; no streaming SSR - **Integration:** Inertia replaces Rails views per-route (all-or-nothing) vs React on Rails' `react_component` helper for incremental adoption in any ERB/Haml template - **Lock-in:** Inertia controllers are coupled to the Inertia protocol; React on Rails uses standard Rails rendering - **Feature gap:** No path to React Server Components or fragment caching ## Validation - `npm run prepare` in `reactonrails.com` - `npm run build` in `reactonrails.com` - `npm run audit:docs` in `reactonrails.com` - Spot-checked rendered pages in local browser ## Notes - This PR is stacked on top of #2650. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Low risk: documentation-only additions/edits with no runtime or API changes; primary risk is broken links or inaccurate guidance. > > **Overview** > Adds new evaluator-focused docs: a narrative decision guide comparing React on Rails with Hotwire/Turbo, Inertia Rails, Next.js+Rails API, react-rails, plus a dedicated page on the Next.js+separate Rails backend tradeoffs. > > Updates the existing alternatives matrix to expand the Inertia section with clearer adoption/performance limitations, and refreshes entry-point docs (`docs/README.md`, `oss/introduction.md`, `quick-start.md`) to link to the new guides and adjust the install instructions toward `--typescript` and `.tsx` examples. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 8ccad36. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Opus 4.6 <[email protected]>
## Summary - rescue top-level `Interrupt` around the overmind/foreman system call so `bin/dev` exits quietly on Ctrl-C - add a generated `bin/shakapacker-watch` wrapper and switch Procfiles to use it for watcher processes - update generator/spec fixtures so the new watcher command is covered in the base and RSC flows ## Why During the fresh-app validation runs, `bin/dev` shut down successfully but printed Ruby interrupt backtraces from both the main process manager and the `server-bundle` watcher. That is a bad first-run experience immediately after following the docs. ## What changed - `ProcessManager.run_process_if_available` now treats Ctrl-C during process-manager shutdown as a clean exit path - new generated `bin/shakapacker-watch` traps `INT`/`TERM`, forwards `TERM` to `bin/shakapacker`, and waits quietly - `Procfile.dev`, `Procfile.dev-static-assets`, and the RSC watcher insertion now use `bin/shakapacker-watch --watch` - updated generator specs and `spec/dummy` Procfiles to reflect the new watcher path - cleaned up `process_manager_spec` to stub `described_class.system` directly instead of using flaky `expect_any_instance_of(Kernel)` expectations ## Validation - `bundle exec rspec react_on_rails/spec/react_on_rails/dev/process_manager_spec.rb` - `bundle exec rspec react_on_rails/spec/react_on_rails/generators/install_generator_spec.rb:39` - `sh -n react_on_rails/lib/generators/react_on_rails/templates/base/base/bin/shakapacker-watch` - manually reproduced the old backtraces in a preserved fresh app, then reran `bin/dev` with the patched gem code + watcher wrapper and confirmed Ctrl-C exits cleanly with no Ruby backtrace ## Notes - This PR is stacked on top of #2650. - I did not run the RSC generator spec line that asserts the Procfile watcher path because the local environment still lacks the matching `react_on_rails_pro (~> 16.4.0)` gem version required by that example group. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Low risk dev-workflow change: adjusts signal/interrupt handling and Procfile watcher commands, with minimal impact outside local development but potential for minor behavior differences in process shutdown. > > **Overview** > Improves local `bin/dev` shutdown by treating `Interrupt` during overmind/foreman execution as a clean exit in `ReactOnRails::Dev::ProcessManager` (avoids noisy backtraces on Ctrl-C). > > Updates generated Procfiles (including RSC watcher insertion) to run watchers via a new `bin/shakapacker-watch` wrapper that traps `INT`/`TERM` and forwards termination to the underlying `bin/shakapacker --watch` process. Generator and dummy/spec fixtures are updated accordingly, and `process_manager_spec` stubs `described_class.system` directly for more reliable expectations. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 951e126. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Enhanced asset watcher process management with improved signal handling and graceful shutdown capabilities for better stability, responsiveness, and control during development workflows * **Bug Fixes** * Improved Ctrl-C interrupt signal handling during development process startup and initialization for faster and more reliable process termination with better developer experience <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
- make `create-react-on-rails-app` match the validated happy path for fresh apps - remove avoidable friction from installing into an existing Rails app - add migration guidance for `vite_rails` and tighten upgrade guidance based on real sample-app runs - pass `--force` through the generator path used by `create-react-on-rails-app` - preserve the requested package manager during generation and normalize pnpm apps after scaffolding - update generator success output and docs to include `bin/rails db:prepare`, PostgreSQL setup, and the current docs URL - warn instead of hard-failing on dirty worktrees during install, while still keeping stricter git checks for release tasks - replace the stock Rails `bin/dev` automatically so fresh existing-app installs stay non-interactive - document current upgrade constraints discovered from validation runs, including Rails 5.2+ and Bundler lockfile refresh steps for older apps - add a new migration guide for `vite_rails` - `pnpm --filter create-react-on-rails-app test` - `pnpm --filter create-react-on-rails-app run build` - `bundle exec rspec react_on_rails/spec/react_on_rails/git_utils_spec.rb` - `bundle exec rspec react_on_rails/spec/react_on_rails/generators/install_generator_spec.rb:2516` - `bundle exec rspec react_on_rails/spec/react_on_rails/generators/install_generator_spec.rb:2527` - validated fresh JS app creation locally through `bin/rails db:prepare`, `bin/dev`, and an HTTP 200 on `/hello_world` - validated fresh Rspack app creation locally through `bin/rails db:prepare`, `bin/dev`, and an HTTP 200 on `/hello_world` - validated install into a fresh Rails app locally through generator run, `bin/rails db:prepare`, `bin/dev`, and an HTTP 200 on `/hello_world` - exercised migration/upgrade docs against open-source sample apps using `react_on_rails`, `react-rails`, and `vite_rails` to capture real blockers - Running the full `install_generator_spec.rb` file currently hits a pre-existing npm/dummy-app install issue in `react_on_rails/spec/react_on_rails/dummy-for-generators`; the newly added examples pass directly. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes scaffolding and install-generator behavior (force overwrites, git checks, bin/dev replacement, package manager normalization), which can affect how existing apps are modified and how installs proceed across environments. > > **Overview** > Improves the *new-app scaffolding path* so `create-react-on-rails-app` runs the Rails install generator non-interactively (adds `--force`), passes the selected package manager through via `REACT_ON_RAILS_PACKAGE_MANAGER`, and post-processes pnpm scaffolds to remove npm artifacts (sets `package.json#packageManager`, converts/removes `package-lock.json`, and updates `bin/setup`). > > Reduces friction when installing into existing Rails apps by softening git worktree checks (warn instead of hard-fail except when Pro/RSC auto-install would mutate a dirty tree), automatically replacing stock Rails `bin/dev` templates while preserving custom ones, and improving platform-safe CLI detection (`which`/`where`). > > Updates docs and user-facing messages to reflect the current happy path (`bin/rails db:prepare`, PostgreSQL note, updated docs URL), fixes a client/server variant docs typo, expands upgrade preflight guidance, and adds a new `vite_rails` migration guide; smoke tests/specs are extended to cover these flows. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 725bcfd. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> * **Documentation** * Updated docs links, added migration guides (Vite→React on Rails), comparison-with-alternatives, clarified upgrade/migration preflight steps, added PostgreSQL prerequisite and explicit "bin/rails db:prepare" after app creation; release/CHANGELOG updated. * **New Features** * Rspack support, TypeScript-first generator (JavaScript option documented), improved package-manager handling (pnpm support + env override), environment-aware bundler detection and env-specific bundler configs. * **Bug Fixes** * Clearer install/doctor messages, safer bin/dev handling, CI-aware git-state warnings. * **Tests** * Expanded coverage for generators, package-manager flows, bundler detection, and git-state behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.6 <[email protected]>
- make `create-react-on-rails-app` match the validated happy path for fresh apps - remove avoidable friction from installing into an existing Rails app - add migration guidance for `vite_rails` and tighten upgrade guidance based on real sample-app runs - pass `--force` through the generator path used by `create-react-on-rails-app` - preserve the requested package manager during generation and normalize pnpm apps after scaffolding - update generator success output and docs to include `bin/rails db:prepare`, PostgreSQL setup, and the current docs URL - warn instead of hard-failing on dirty worktrees during install, while still keeping stricter git checks for release tasks - replace the stock Rails `bin/dev` automatically so fresh existing-app installs stay non-interactive - document current upgrade constraints discovered from validation runs, including Rails 5.2+ and Bundler lockfile refresh steps for older apps - add a new migration guide for `vite_rails` - `pnpm --filter create-react-on-rails-app test` - `pnpm --filter create-react-on-rails-app run build` - `bundle exec rspec react_on_rails/spec/react_on_rails/git_utils_spec.rb` - `bundle exec rspec react_on_rails/spec/react_on_rails/generators/install_generator_spec.rb:2516` - `bundle exec rspec react_on_rails/spec/react_on_rails/generators/install_generator_spec.rb:2527` - validated fresh JS app creation locally through `bin/rails db:prepare`, `bin/dev`, and an HTTP 200 on `/hello_world` - validated fresh Rspack app creation locally through `bin/rails db:prepare`, `bin/dev`, and an HTTP 200 on `/hello_world` - validated install into a fresh Rails app locally through generator run, `bin/rails db:prepare`, `bin/dev`, and an HTTP 200 on `/hello_world` - exercised migration/upgrade docs against open-source sample apps using `react_on_rails`, `react-rails`, and `vite_rails` to capture real blockers - Running the full `install_generator_spec.rb` file currently hits a pre-existing npm/dummy-app install issue in `react_on_rails/spec/react_on_rails/dummy-for-generators`; the newly added examples pass directly. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes scaffolding and install-generator behavior (force overwrites, git checks, bin/dev replacement, package manager normalization), which can affect how existing apps are modified and how installs proceed across environments. > > **Overview** > Improves the *new-app scaffolding path* so `create-react-on-rails-app` runs the Rails install generator non-interactively (adds `--force`), passes the selected package manager through via `REACT_ON_RAILS_PACKAGE_MANAGER`, and post-processes pnpm scaffolds to remove npm artifacts (sets `package.json#packageManager`, converts/removes `package-lock.json`, and updates `bin/setup`). > > Reduces friction when installing into existing Rails apps by softening git worktree checks (warn instead of hard-fail except when Pro/RSC auto-install would mutate a dirty tree), automatically replacing stock Rails `bin/dev` templates while preserving custom ones, and improving platform-safe CLI detection (`which`/`where`). > > Updates docs and user-facing messages to reflect the current happy path (`bin/rails db:prepare`, PostgreSQL note, updated docs URL), fixes a client/server variant docs typo, expands upgrade preflight guidance, and adds a new `vite_rails` migration guide; smoke tests/specs are extended to cover these flows. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 725bcfd. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> * **Documentation** * Updated docs links, added migration guides (Vite→React on Rails), comparison-with-alternatives, clarified upgrade/migration preflight steps, added PostgreSQL prerequisite and explicit "bin/rails db:prepare" after app creation; release/CHANGELOG updated. * **New Features** * Rspack support, TypeScript-first generator (JavaScript option documented), improved package-manager handling (pnpm support + env override), environment-aware bundler detection and env-specific bundler configs. * **Bug Fixes** * Clearer install/doctor messages, safer bin/dev handling, CI-aware git-state warnings. * **Tests** * Expanded coverage for generators, package-manager flows, bundler detection, and git-state behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.6 <[email protected]>
1 participant
Summary
create-react-on-rails-appmatch the validated happy path for fresh appsvite_railsand tighten upgrade guidance based on real sample-app runsWhat changed
--forcethrough the generator path used bycreate-react-on-rails-appbin/rails db:prepare, PostgreSQL setup, and the current docs URLbin/devautomatically so fresh existing-app installs stay non-interactivevite_railsValidation
pnpm --filter create-react-on-rails-app testpnpm --filter create-react-on-rails-app run buildbundle exec rspec react_on_rails/spec/react_on_rails/git_utils_spec.rbbundle exec rspec react_on_rails/spec/react_on_rails/generators/install_generator_spec.rb:2516bundle exec rspec react_on_rails/spec/react_on_rails/generators/install_generator_spec.rb:2527bin/rails db:prepare,bin/dev, and an HTTP 200 on/hello_worldbin/rails db:prepare,bin/dev, and an HTTP 200 on/hello_worldbin/rails db:prepare,bin/dev, and an HTTP 200 on/hello_worldreact_on_rails,react-rails, andvite_railsto capture real blockersNotes
install_generator_spec.rbfile currently hits a pre-existing npm/dummy-app install issue inreact_on_rails/spec/react_on_rails/dummy-for-generators; the newly added examples pass directly.Note
Medium Risk
Changes scaffolding and install-generator behavior (force overwrites, git checks, bin/dev replacement, package manager normalization), which can affect how existing apps are modified and how installs proceed across environments.
Overview
Improves the new-app scaffolding path so
create-react-on-rails-appruns the Rails install generator non-interactively (adds--force), passes the selected package manager through viaREACT_ON_RAILS_PACKAGE_MANAGER, and post-processes pnpm scaffolds to remove npm artifacts (setspackage.json#packageManager, converts/removespackage-lock.json, and updatesbin/setup).Reduces friction when installing into existing Rails apps by softening git worktree checks (warn instead of hard-fail except when Pro/RSC auto-install would mutate a dirty tree), automatically replacing stock Rails
bin/devtemplates while preserving custom ones, and improving platform-safe CLI detection (which/where).Updates docs and user-facing messages to reflect the current happy path (
bin/rails db:prepare, PostgreSQL note, updated docs URL), fixes a client/server variant docs typo, expands upgrade preflight guidance, and adds a newvite_railsmigration guide; smoke tests/specs are extended to cover these flows.Written by Cursor Bugbot for commit 725bcfd. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
Documentation
New Features
Bug Fixes
Tests