Add --rsc flow to create-react-on-rails-app#2430
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:
WalkthroughAdds React Server Components (RSC) support to the create-react-on-rails-app CLI: new Changes
Sequence DiagramsequenceDiagram
participant User
participant CLI as "create-react-on-rails-app CLI"
participant Args as "buildGeneratorArgs()"
participant Creator as "createApp"
participant Bundler as "Bundler / Gem installer"
participant UX as "Success printer"
User->>CLI: run generator with --rsc and app name
CLI->>Args: buildGeneratorArgs(options with rsc=true)
Args-->>CLI: return generator args (includes --rsc)
CLI->>Creator: invoke createApp with args
Creator->>Bundler: install react_on_rails
alt rsc enabled
Creator->>Bundler: canResolveRemoteGem("react_on_rails_pro")
Bundler-->>Creator: available / not available
Creator->>Bundler: install react_on_rails_pro
end
Bundler-->>Creator: installation complete
Creator->>UX: print success with route "/hello_server"
UX-->>User: display final instructions with /hello_server
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
size-limit report 📦
|
Greptile SummaryAdds
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant CLI as create-react-on-rails-app
participant Rails as rails new
participant Bundler as bundle add
participant Generator as rails g react_on_rails:install
User->>CLI: npx create-react-on-rails-app my-app --rsc
CLI->>CLI: Validate app name & prerequisites
CLI->>Rails: rails new my-app --database=postgresql --skip-javascript
Rails-->>CLI: Rails app created
CLI->>Bundler: bundle add react_on_rails --strict
Bundler-->>CLI: react_on_rails added
Note over CLI,Bundler: --rsc triggers Pro gem install
CLI->>Bundler: bundle add react_on_rails_pro --strict
Bundler-->>CLI: react_on_rails_pro added
CLI->>Generator: bundle exec rails generate react_on_rails:install --rsc --ignore-warnings
Note over Generator: use_pro? returns true (rsc implies pro)
Generator-->>CLI: Pro + RSC setup complete
CLI-->>User: Visit http://localhost:3000/hello_server
Last reviewed commit: 104b4c5 |
![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
🤖 Fix all issues with AI agents
In `@packages/create-react-on-rails-app/src/create-app.ts`:
- Around line 87-89: When options.rsc is true, the code directly calls
execLiveArgs('bundle', ['add', 'react_on_rails_pro', '--strict'], appPath) which
will fail for users who don't have the private react_on_rails_pro source
configured; add a preflight validation before that call in create-app.ts that
attempts to resolve the gem (for example by running a probe command like `gem
list -r react_on_rails_pro` or `bundle info react_on_rails_pro` and checking its
exit code) and if the gem cannot be resolved, abort the add and emit a clear,
actionable error via your logger explaining the required prerequisites
(configure private gem server or add a git-based Gemfile entry) and how to
proceed; update the branch around the execLiveArgs call and the accompanying
logStepDone message to only run when validation succeeds.
🧹 Nitpick comments (1)
docs/getting-started/create-react-on-rails-app.md (1)
43-43: Nit: The--rscoption description is slightly self-referential."Generate RSC setup (
--rscinstall flow)" restates the flag name rather than explaining what it does. Consider something like "Enable React Server Components (requiresreact_on_rails_pro)" for clarity.Suggested wording
-| `--rsc` | Generate RSC setup (`--rsc` install flow) | `false` | +| `--rsc` | Enable React Server Components (requires `react_on_rails_pro`) | `false` |
justin808
force-pushed
the
codex/add-rsc-option-to-create-react-app
branch
from
ff678f6 to
cad1f6b
Compare
Review: Add --rsc flow to create-react-on-rails-appThe PR is clean and well-structured. The approach is straightforward and the tests added for Issues1. Partial app state on If
The error message says 2. No preemptive warning about Pro gem access The current flow only tells users about the Pro gem requirement when it fails. Since 3. Compatibility between Both flags can be passed together ( 4. Missing tests for the The new tests cover
5. Minor: Static help text inconsistency (index.ts L105) The static This is technically accurate, but the first line will read as a general instruction and could mislead RSC users into expecting SummaryThe core feature logic is correct. Items 1 and 2 are the most impactful for user experience — partial state on failure is confusing, and no upfront warning about a commercial dependency is a potential surprise. Items 3–5 are lower priority but worth addressing. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/create-react-on-rails-app/tests/create-app.test.ts (1)
68-104: Consider adding arspack + rsc(no TypeScript) combination test.The existing suite covers each flag independently and all three together, but the
rspack: true, rsc: true, template: 'javascript'pairing — a realistic user workflow — isn't explicitly exercised.✅ Suggested additional test case
it('combines all enabled flags in order', () => { expect( buildGeneratorArgs({ ...baseOptions, template: 'typescript', rspack: true, rsc: true, }), ).toEqual(['--typescript', '--rspack', '--rsc', '--ignore-warnings']); }); + + it('combines rspack and rsc flags without typescript', () => { + expect( + buildGeneratorArgs({ ...baseOptions, rspack: true, rsc: true }), + ).toEqual(['--rspack', '--rsc', '--ignore-warnings']); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/create-react-on-rails-app/tests/create-app.test.ts` around lines 68 - 104, Add a new unit test in the create-app.test.ts suite for buildGeneratorArgs that covers the rspack + rsc combination with a javascript template (i.e., { ...baseOptions, rspack: true, rsc: true, template: 'javascript' }) and assert the returned array equals ['--rspack','--rsc','--ignore-warnings']; place it alongside the existing cases so the ordering behavior is verified for this realistic flag pairing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/create-react-on-rails-app/tests/create-app.test.ts`:
- Around line 68-104: Add a new unit test in the create-app.test.ts suite for
buildGeneratorArgs that covers the rspack + rsc combination with a javascript
template (i.e., { ...baseOptions, rspack: true, rsc: true, template:
'javascript' }) and assert the returned array equals
['--rspack','--rsc','--ignore-warnings']; place it alongside the existing cases
so the ordering behavior is verified for this realistic flag pairing.
cad1f6b to
4ecb98a
Compare
|
Addressed all review comments on this PR:
|
|
PR Review summary — see inline comments for details and code suggestions. Bug 1 (blocking): canResolveRemoteGem always returns false (utils.ts:27-32). gem list --remote --exact prints a header line before the gem listing. After .trim() neither the startsWith nor equality check matches the gem name, so the preflight always fails and --rsc is completely unusable. Fix: add the --quiet flag to the gem list call. Bug 2 (blocking): gem and bundle use separate source registries (utils.ts:27). gem list --remote queries ~/.gemrc sources; bundle add uses Bundler config. Users with Bundler-based access to the private gem are rejected by the preflight even though bundle add would succeed. Minor: --rsc combined with --template javascript is not validated (index.ts:33). RSC requires TypeScript but the CLI accepts this combination silently and fails inside the generator. An early guard or auto-upgrade prevents user confusion. Positive notes: preflight before Rails app creation avoids a half-baked directory on auth failure; error messaging is helpful; the buildGeneratorArgs and createApp test suites are clean and cover the right scenarios. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/getting-started/create-react-on-rails-app.md (1)
16-38: Add package-manager-agnostic invocation examples for end-user docs.The new command examples are
npx-only. Please also show Yarn and pnpm equivalents (for example,yarn create/pnpm dlx) in Quick Start or Options examples for consistency across docs.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/getting-started/create-react-on-rails-app.md` around lines 16 - 38, Update the example invocations for create-react-on-rails-app so they are package-manager-agnostic: for every npx create-react-on-rails-app example (including the RSC, --template javascript, --rspack, --package-manager pnpm, and combined-flag examples) add the equivalent yarn and pnpm variants (yarn create react-on-rails-app ... and pnpm dlx create-react-on-rails-app ...), and ensure the “For React Server Components (RSC)” line mentions the three ways to run the command (npm/npx, yarn, pnpm) so all Options examples consistently show npm, yarn, and pnpm forms.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/getting-started/create-react-on-rails-app.md`:
- Around line 16-38: Update the example invocations for
create-react-on-rails-app so they are package-manager-agnostic: for every npx
create-react-on-rails-app example (including the RSC, --template javascript,
--rspack, --package-manager pnpm, and combined-flag examples) add the equivalent
yarn and pnpm variants (yarn create react-on-rails-app ... and pnpm dlx
create-react-on-rails-app ...), and ensure the “For React Server Components
(RSC)” line mentions the three ways to run the command (npm/npx, yarn, pnpm) so
all Options examples consistently show npm, yarn, and pnpm forms.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a757ceb4-9ffc-4def-8f24-a4e77c3751d3
📒 Files selected for processing (7)
CHANGELOG.mddocs/getting-started/create-react-on-rails-app.mdpackages/create-react-on-rails-app/src/create-app.tspackages/create-react-on-rails-app/src/index.tspackages/create-react-on-rails-app/src/types.tspackages/create-react-on-rails-app/src/utils.tspackages/create-react-on-rails-app/tests/create-app.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/create-react-on-rails-app/src/index.ts
- packages/create-react-on-rails-app/src/types.ts
| logStep(4, TOTAL_STEPS, 'Done!'); | ||
| printSuccessMessage(appName); | ||
| // Final success | ||
| logStepDone('Done!'); |
There was a problem hiding this comment.
Redundant logStepDone call
logStepDone('React on Rails setup complete') is printed on line 197 (inside the try block immediately after the generator finishes), and then logStepDone('Done!') fires here on line 213 unconditionally. The user sees two consecutive green-checkmark lines with no actionable distinction between them.
The original code had a dedicated logStep(4, 4, 'Done!') numbered step which felt intentional. If the intention is to keep a final marker, consider removing the inner logStepDone('React on Rails setup complete') from the try block and keeping only this one, or simply dropping logStepDone('Done!') entirely and letting printSuccessMessage serve as the completion signal.
| logStepDone('Done!'); | |
| printSuccessMessage(appName, options.rsc ? 'hello_server' : 'hello_world'); |
| @@ -60,11 +108,16 @@ export function validateAppName(name: string): { success: boolean; error?: strin | |||
|
|
|||
| export function createApp(appName: string, options: CliOptions): void { | |||
There was a problem hiding this comment.
No preflight check before starting the multi-step process for --rsc
When --rsc is used, the user runs through the full rails new + bundle add react_on_rails sequence before hitting the react_on_rails_pro installation step (step 3). If Pro isn't accessible from their Bundler/RubyGems setup — a likely scenario for new users who haven't configured a private gem server — they waste time on the first two steps, then get a cleanup and error message.
An earlier version of this PR's walkthrough (per CodeRabbit) mentioned a canResolveRemoteGem preflight in utils.ts, but that check doesn't appear in the current diff. Adding a pre-flight probe (even a simple bundle info react_on_rails_pro --no-color / gem search react_on_rails_pro) before rails new would save users significant frustration on a bad --rsc setup.
If adding the probe is out of scope for this PR, a clear note in the --rsc info block (already shown in index.ts) linking to Pro access/configuration is the minimum, and that is already there — but the failure UX after two successful steps is still rough.
| test -f "$APP_RSC_JS_DIR/app/javascript/src/HelloServer/components/HelloServer.jsx" | ||
| grep -q "stream_react_component('HelloServer'" "$APP_RSC_JS_DIR/app/views/hello_server/index.html.erb" | ||
|
|
||
| grep -q "gem \"react_on_rails_pro\"" "$APP_RSC_TS_DIR/Gemfile" |
There was a problem hiding this comment.
Smoke test assertions couple tightly to generator output paths
Lines like:
test -f "$APP_RSC_JS_DIR/app/javascript/src/HelloServer/components/HelloServer.jsx"
grep -q "stream_react_component('HelloServer'" "$APP_RSC_JS_DIR/app/views/hello_server/index.html.erb"hard-code specific file paths and template content that come from the RSC generator (in react_on_rails_pro). If the generator renames files or changes the helper call, these assertions will fail without obvious cause.
Consider adding a brief comment explaining where these expected paths come from so maintainers know to update the smoke test when the generator changes:
# Expected files generated by react_on_rails:install --rsc (from react_on_rails_pro generator)
test -f "$APP_RSC_JS_DIR/app/javascript/src/HelloServer/components/HelloServer.jsx"
Review SummaryOverall this is a clean, well-structured addition. The step counter logic is clear, error paths all clean up the app directory, unit tests are thorough, and the smoke test script is a nice contribution for local development. A few things worth addressing: Issue 1 - No preflight check for react_on_rails_pro availability Issue 2 - react_on_rails_pro installed without --strict (undocumented intent) Issue 3 - Double logStepDone after generator completes Minor - Smoke test assertions tightly coupled to generator output What is good
|
1 participant
Summary
--rscCLI flag tocreate-react-on-rails-app--rscin generatedrails generate react_on_rails:installargsreact_on_rails_proautomatically when--rscis selected/hello_serverfor RSC scaffoldscreate-react-on-rails-appoptions and flow--rsccombinations)Why
Current CLI scaffolding cannot generate RSC apps directly, even though the install generator supports
--rsc. This forces manual steps and creates confusion in docs and onboarding.Verification
mise exec node@22 -- corepack pnpm --filter create-react-on-rails-app testmise exec node@22 -- corepack pnpm --filter create-react-on-rails-app type-checkmise exec node@22 -- corepack pnpm exec eslint packages/create-react-on-rails-app/src/create-app.ts packages/create-react-on-rails-app/src/index.ts packages/create-react-on-rails-app/src/types.tsSummary by CodeRabbit
Note
Medium Risk
Moderate risk because it changes the scaffolding/install flow (adds a new gem install path and generator flags) and introduces a failure mode if Pro gem access is misconfigured.
Overview
Adds an
--rscmode tocreate-react-on-rails-app, passing--rscthrough torails generate react_on_rails:install, installingreact_on_rails_prowhen selected, and updating the success output to direct users to/hello_server(vs/hello_world).Updates CLI help text, docs, and the changelog to document the new flag and flow, and adds unit tests around
buildGeneratorArgsto cover flag combinations (including ordering).Written by Cursor Bugbot for commit cad1f6b. Configure here.