Improve create-react-on-rails-app fresh app onboarding#2849
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
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 fresh-app onboarding: generated apps include a root landing page at Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI (create-app)
participant Git as Git
participant RailsGen as Rails generator
participant Server as ServerManager
CLI->>Git: ensure repository exists / read user.name & user.email
CLI->>RailsGen: run `rails new` + generator args (--new-app, --rsc/--pro...)
RailsGen-->>CLI: scaffold (may inject root route, HomeController, landing page)
CLI->>Git: createEducationalCommit("rails new")
CLI->>CLI: bundle add react_on_rails (and optionally react_on_rails_pro)
CLI->>Git: createEducationalCommit("add react_on_rails / pro")
CLI->>RailsGen: run react_on_rails generator (--new-app)
CLI->>Git: createEducationalCommit("run generators / scaffold")
CLI->>Server: print success message (visit http://localhost:3000) + git-history hint
Note right of Server: on first bin/dev run, ServerManager may poll port and open browser once
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 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 docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsTimed out fetching pipeline failures after 30000ms 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}
Greptile SummaryThis PR significantly improves the fresh-app onboarding experience: Key observations:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant CLI as create-react-on-rails-app
participant Git as Git
participant Rails as rails new
participant Bundle as Bundler
participant Gen as react_on_rails:install --new-app
participant Dev as bin/dev (thread)
participant Browser as Browser
CLI->>CLI: validateAll() — includes validateGit()
CLI->>Rails: rails new --database=postgresql --skip-javascript
Rails-->>CLI: app directory created + git init
CLI->>Git: createEducationalCommit("Create Rails app with PostgreSQL")
CLI->>Bundle: bundle add react_on_rails
CLI->>Git: createEducationalCommit("Add react_on_rails gem")
opt --rsc or --pro
CLI->>Bundle: bundle add react_on_rails_pro
CLI->>Git: createEducationalCommit("Add react_on_rails_pro gem")
end
CLI->>Gen: rails generate react_on_rails:install --new-app [flags]
Gen-->>CLI: home_controller.rb + index.html.erb.tt + root route + bin/dev (AUTO_OPEN_BROWSER_ONCE=true)
CLI->>Git: createEducationalCommit("Install React on Rails with …")
opt packageManager == pnpm
CLI->>CLI: normalizeGeneratedPackageManager (pnpm import + install)
CLI->>Git: createEducationalCommit("Normalize the generated app for pnpm")
end
Note over CLI: User runs bin/dev
Dev->>Dev: inject --open-browser-once into argv (AUTO_OPEN_BROWSER_ONCE=true)
Dev->>Dev: schedule_browser_open thread spawned
Dev->>Dev: ProcessManager.run_with_process_manager (system() — thread survives)
Dev-->>Browser: wait_for_server_on_port → open_browser(url)
Dev->>Dev: mark_browser_opened_once (writes tmp/react_on_rails/browser_opened_once)
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Baseline educational commit silently skipped when git configured
- Added
--skip-gittorails newso the CLI always owns repo initialization and reliably creates the baseline educational commit.
- Added
- ✅ Fixed: Git commit failure deletes app and shows wrong error
- Wrapped educational commit creation in its own try/catch with a warning log so git commit failures no longer trigger scaffold-step cleanup and misleading fatal errors.
Or push these changes by commenting:
@cursor push f6ad2a0abd
Preview (f6ad2a0abd)
diff --git a/packages/create-react-on-rails-app/src/create-app.ts b/packages/create-react-on-rails-app/src/create-app.ts
--- a/packages/create-react-on-rails-app/src/create-app.ts
+++ b/packages/create-react-on-rails-app/src/create-app.ts
@@ -211,14 +211,19 @@
}
function createEducationalCommit(appPath: string, subject: string, body: string): void {
- ensureGitRepository(appPath);
+ try {
+ ensureGitRepository(appPath);
- if (!gitHasPendingChanges(appPath)) {
- return;
+ if (!gitHasPendingChanges(appPath)) {
+ return;
+ }
+
+ execLiveArgs('git', ['add', '-A'], appPath);
+ execLiveArgs('git', ['commit', '-m', subject, '-m', body], appPath, educationalGitEnv(appPath));
+ } catch (error) {
+ const message = error instanceof Error ? error.message : 'unknown git error';
+ logInfo(`Skipping educational commit "${subject}" due to git error: ${message}`);
}
-
- execLiveArgs('git', ['add', '-A'], appPath);
- execLiveArgs('git', ['commit', '-m', subject, '-m', body], appPath, educationalGitEnv(appPath));
}
function railsBaselineCommitMessage(): { subject: string; body: string } {
@@ -351,7 +356,7 @@
// so it's always a simple directory name safe to use with rails new.
logStep(currentStep, totalSteps, 'Creating Rails application...');
try {
- execLiveArgs('rails', ['new', appName, '--database=postgresql', '--skip-javascript']);
+ execLiveArgs('rails', ['new', appName, '--database=postgresql', '--skip-javascript', '--skip-git']);
const { subject, body } = railsBaselineCommitMessage();
createEducationalCommit(appPath, subject, body);
logStepDone('Rails application created');
diff --git a/packages/create-react-on-rails-app/tests/create-app.test.ts b/packages/create-react-on-rails-app/tests/create-app.test.ts
--- a/packages/create-react-on-rails-app/tests/create-app.test.ts
+++ b/packages/create-react-on-rails-app/tests/create-app.test.ts
@@ -270,6 +270,7 @@
'my-app',
'--database=postgresql',
'--skip-javascript',
+ '--skip-git',
]);
expect(mockedExecLiveArgs).toHaveBeenNthCalledWith(
4,
@@ -309,6 +310,7 @@
'my-app',
'--database=postgresql',
'--skip-javascript',
+ '--skip-git',
]);
expect(mockedExecLiveArgs).toHaveBeenNthCalledWith(
4,
@@ -365,6 +367,7 @@
'my-app',
'--database=postgresql',
'--skip-javascript',
+ '--skip-git',
]);
expect(mockedExecLiveArgs).toHaveBeenNthCalledWith(
4,
@@ -391,6 +394,23 @@
expect(processExitSpy).not.toHaveBeenCalled();
});
+ it('continues app creation when an educational commit fails', () => {
+ const appPath = path.resolve(process.cwd(), 'my-app');
+ mockedExecLiveArgs.mockImplementation((command, args) => {
+ if (command === 'git' && args[0] === 'commit' && args[2] === 'Create Rails app with PostgreSQL') {
+ throw new Error('hook rejected commit');
+ }
+ });
+
+ createApp('my-app', baseOptions);
+
+ expect(processExitSpy).not.toHaveBeenCalled();
+ expect(mockedFs.rmSync).not.toHaveBeenCalledWith(appPath, { recursive: true, force: true });
+ expect(mockedLogInfo).toHaveBeenCalledWith(
+ expect.stringContaining('Skipping educational commit "Create Rails app with PostgreSQL" due to git error'),
+ );
+ });
+
it('converts pnpm scaffolds away from npm artifacts', () => {
const appPath = path.resolve(process.cwd(), 'my-app');
const packageJsonPath = path.join(appPath, 'package.json');
AGENTS.mdInstructions for AI coding agents working on the React on Rails codebase. React on Rails is a Ruby gem + npm package that integrates React with Ruby on Rails, providing server-side rendering (SSR) via Node.js or ExecJS. This is a monorepo: the open-source gem lives at Reusable Workflows
Canonical Agent Policy
Other agent-facing docs (for example Commands# Install dependencies
bundle && pnpm install
# Build TypeScript → JavaScript
pnpm run build
# Lint (MANDATORY before every commit)
bundle exec rubocop # Ruby — must pass with zero offenses
pnpm run lint # JS/TS via ESLint
pnpm start format.listDifferent # Check Prettier formatting
rake lint # All linting (Ruby + JS + formatting)
# Auto-fix formatting
rake autofix # Preferred for all formatting
# Run tests
rake run_rspec:gem # Ruby unit tests (gem code)
rake run_rspec:dummy # Ruby integration tests (dummy Rails app)
pnpm run test # JavaScript/TypeScript tests
rake # Full suite (lint + all tests except examples)
# Type checking
pnpm run type-check # TypeScript
bundle exec rake rbs:validate # RBS signatures
# Additional test subsets
rake run_rspec # All Ruby tests
rake all_but_examples # All tests except generated examples
rake run_rspec:shakapacker_examples_basic # Single example test
# Full initial setup
bundle && pnpm install && rake shakapacker_examples:gen_all && rake node_package && rake
# CI/workflow linting
actionlint # GitHub Actions lint
yamllint .github/ # YAML lint (do NOT run RuboCop on .yml files)
# Dependency version updates
rake shakapacker:update_version[9.6.1] # Update shakapacker across the monorepoUpdating ShakapackerUse The task handles Ruby version switching for apps that require a different Ruby version (set Testing
Run specific test files: bundle exec rspec react_on_rails/spec/react_on_rails/path/to/spec.rb
cd react_on_rails/spec/dummy && bundle exec rspec spec/path/to/spec.rbProject Structure
Code StyleRuby (RuboCop)Line length max 120 characters. Run Line length — break long chains: # Bad
content = pack_content.gsub(/import.*from.*['"];/, "").gsub(/ReactOnRails\.register.*/, "")
# Good
content = pack_content.gsub(/import.*from.*['"];/, "")
.gsub(/ReactOnRails\.register.*/, "")Named subjects in RSpec: # Bad
subject { instance.method_name(arg) }
# Good
subject(:method_result) { instance.method_name(arg) }Security violations — scope disable comments tightly: # rubocop:disable Security/Eval
expect { evaluate(sanitized_content) }.not_to raise_error
# rubocop:enable Security/EvalJavaScript/TypeScriptPrettier handles all formatting. Never manually format — run Git WorkflowBranch naming: Commit messages: Explain why, not what. One logical change per commit. PR creation: Use Review WorkflowFor small, focused PRs (roughly 5 files changed or fewer and one clear purpose):
BoundariesAlways
Ask First
Never
Key Concept: File Suffixes vs. RSC DirectiveReact on Rails has two independent systems that both use "client" and "server" terminology. Do not confuse them. 1. Bundle Placement (
|
PR Review: Improve create-react-on-rails-app fresh app onboardingOverall this is a well-structured feature. The generated landing page, educational git history, and browser auto-open are all valuable developer UX improvements. Good test coverage across the new code paths. A few issues worth addressing before merging: Bug: explicit --open-browser silently no-ops in non-TTY contextsbrowser_auto_open_allowed? checks stdin.tty? and stdout.tty?, which makes sense for the auto/generated AUTO_OPEN_BROWSER_ONCE behavior. But schedule_browser_open applies this same guard for both the implicit case (generated constant) and the explicit case (user typed --open-browser). In a Docker container, an SSH session without a pseudo-TTY, or when output is piped, the flag silently does nothing with no warning or error. The TTY guard should only apply to the auto-open path, not to explicit user opt-in. Consider threading an explicit: keyword through schedule_browser_open that bypasses the TTY check. Bug: OPEN_BROWSER_ONCE_MARKER is a relative pathThis constant resolves against Dir.pwd at constant-evaluation time (when the file is first required). If Dir.pwd is not the Rails app root at that moment the marker lands in the wrong directory and the once-open guard silently misfires. Prefer resolving it lazily at call time, e.g. relative to Rails.root. Smoke test gap: JS-only template has no git-history assertionexpect_git_history is called for the TypeScript and RSC-JS variants but not for the plain-JS, Rspack-only, RSC-TS, or RSC-Rspack variants. A commit-message string change on those paths would go undetected. Consider adding expect_git_history for at least the JS-only variant to cover the non-TypeScript branch. Nit: unreachable npm branch in packageManagerCommitMessageThe function constructs a body for the non-pnpm case, but the call site is guarded by a pnpm check - the npm branch is dead code. Either remove it from the function or add a comment at the call site explaining why npm does not get a normalization commit. Nit: Windows start empty-string idiom deserves a commentbrowser_command returns cmd /c start with an empty string on Windows. The empty string is the required window-title placeholder that keeps start from misinterpreting the URL as the title. This is correct but non-obvious - a brief inline comment would save the next reader from a grep detour. |
AGENTS.mdInstructions for AI coding agents working on the React on Rails codebase. React on Rails is a Ruby gem + npm package that integrates React with Ruby on Rails, providing server-side rendering (SSR) via Node.js or ExecJS. This is a monorepo: the open-source gem lives at Reusable Workflows
Canonical Agent Policy
Other agent-facing docs (for example Commands# Install dependencies
bundle && pnpm install
# Build TypeScript → JavaScript
pnpm run build
# Lint (MANDATORY before every commit)
bundle exec rubocop # Ruby — must pass with zero offenses
pnpm run lint # JS/TS via ESLint
pnpm start format.listDifferent # Check Prettier formatting
rake lint # All linting (Ruby + JS + formatting)
# Auto-fix formatting
rake autofix # Preferred for all formatting
# Run tests
rake run_rspec:gem # Ruby unit tests (gem code)
rake run_rspec:dummy # Ruby integration tests (dummy Rails app)
pnpm run test # JavaScript/TypeScript tests
rake # Full suite (lint + all tests except examples)
# Type checking
pnpm run type-check # TypeScript
bundle exec rake rbs:validate # RBS signatures
# Additional test subsets
rake run_rspec # All Ruby tests
rake all_but_examples # All tests except generated examples
rake run_rspec:shakapacker_examples_basic # Single example test
# Full initial setup
bundle && pnpm install && rake shakapacker_examples:gen_all && rake node_package && rake
# CI/workflow linting
actionlint # GitHub Actions lint
yamllint .github/ # YAML lint (do NOT run RuboCop on .yml files)
# Dependency version updates
rake shakapacker:update_version[9.6.1] # Update shakapacker across the monorepoUpdating ShakapackerUse The task handles Ruby version switching for apps that require a different Ruby version (set Testing
Run specific test files: bundle exec rspec react_on_rails/spec/react_on_rails/path/to/spec.rb
cd react_on_rails/spec/dummy && bundle exec rspec spec/path/to/spec.rbProject Structure
Code StyleRuby (RuboCop)Line length max 120 characters. Run Line length — break long chains: # Bad
content = pack_content.gsub(/import.*from.*['"];/, "").gsub(/ReactOnRails\.register.*/, "")
# Good
content = pack_content.gsub(/import.*from.*['"];/, "")
.gsub(/ReactOnRails\.register.*/, "")Named subjects in RSpec: # Bad
subject { instance.method_name(arg) }
# Good
subject(:method_result) { instance.method_name(arg) }Security violations — scope disable comments tightly: # rubocop:disable Security/Eval
expect { evaluate(sanitized_content) }.not_to raise_error
# rubocop:enable Security/EvalJavaScript/TypeScriptPrettier handles all formatting. Never manually format — run Git WorkflowBranch naming: Commit messages: Explain why, not what. One logical change per commit. PR creation: Use Review WorkflowFor small, focused PRs (roughly 5 files changed or fewer and one clear purpose):
BoundariesAlways
Ask First
Never
Key Concept: File Suffixes vs. RSC DirectiveReact on Rails has two independent systems that both use "client" and "server" terminology. Do not confuse them. 1. Bundle Placement (
|
|
PR Review: Improve create-react-on-rails-app fresh app onboarding Overall this is a well-structured, thoughtful onboarding improvement. The layered design — git-validation → educational commits → landing page → first-run browser open — is clean, and the guard logic (CI detection, TTY check, Blocker: GPG commit signing hangs scaffolding Developers with Medium: WSL users get no browser open and no feedback
Medium: Thread.new + report_on_exception = false + next The combination of suppressed exceptions and the non-idiomatic Medium: Brittle positional mock chain in tests The Minor: Memoize educationalGitEnv
Minor: Soft race condition in --open-browser-once The marker-file existence check happens before the thread is spawned; the write happens inside the thread. Two near-simultaneous Observations (no action needed)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
react_on_rails/spec/react_on_rails/generators/install_generator_spec.rb (1)
73-96: Add the preexisting-root-route regression case.These contexts only cover the happy path. Please add one case where
config/routes.rbalready has a non-commentedrootand assert we do not injectroot to: "home#index"or scaffoldHomeController/app/views/home, since that guard is what keeps--new-appfrom clobbering existing apps.Also applies to: 1511-1532
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@react_on_rails/spec/react_on_rails/generators/install_generator_spec.rb` around lines 73 - 96, Add a regression spec that simulates a preexisting non-commented root route and asserts the generator does not inject or scaffold over it: reuse the existing context "with --new-app" and run_generator_test_with_args(%w[--new-app], package_json: true) but with config/routes.rb prepopulated with an existing root (e.g. 'root to: "existing#home"'); then assert_file "config/routes.rb" does NOT include 'root to: "home#index"' and assert_no_file for "app/controllers/home_controller.rb" and "app/views/home/index.html.erb" (or assert those files are unchanged) to confirm HomeController and view are not created when a root already exists.packages/create-react-on-rails-app/scripts/smoke-test-local-gems.sh (1)
126-176: Smoke-test the other commit-history variants too.
expect_git_historyis only used forAPP_TS_DIRandAPP_RSC_JS_DIR. The CLI builds different commit subjects for JavaScript/Rspack/RSC+Rspack flows inpackages/create-react-on-rails-app/src/create-app.ts, so regressions in those variants can slip through this smoke test today.Also applies to: 178-204
🤖 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 126 - 176, The test only asserts commit history for APP_TS_DIR and APP_RSC_JS_DIR, so add expect_git_history checks for the other generated-app variants (APP_JS_DIR and APP_RSPACK_DIR) to catch commit-message regressions; update the smoke-test script to call expect_git_history for APP_JS_DIR and APP_RSPACK_DIR with the expected sequence of commit subjects used by create-app.ts (matching the patterns used for APP_TS_DIR and APP_RSC_JS_DIR), and mirror the same addition in the later block referenced around lines 178-204 so all four app variants (APP_JS_DIR, APP_TS_DIR, APP_RSPACK_DIR, APP_RSC_JS_DIR) have expect_git_history assertions.packages/create-react-on-rails-app/src/create-app.ts (1)
179-222: Consider exception handling ingitHasPendingChanges.Unlike
readGitConfig(which wrapsexecCaptureArgsin try/catch),gitHasPendingChangespropagates exceptions. Ifgit status --porcelainfails (e.g., corrupted git repo), the exception bubbles up throughcreateEducationalCommitto the caller's try/catch, failing the entire step.This is likely intentional (fail fast on git issues), but if you want educational commits to be best-effort without blocking app creation, consider wrapping with try/catch:
Optional: Make git status check resilient
function gitHasPendingChanges(appPath: string): boolean { + try { return execCaptureArgs('git', ['status', '--porcelain'], appPath) !== ''; + } catch { + return false; + } }🤖 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 179 - 222, gitHasPendingChanges currently calls execCaptureArgs('git', ['status', '--porcelain'], appPath) and lets any exception bubble up; make it resilient by wrapping the execCaptureArgs call in a try/catch and returning false on error so educational commits remain best-effort (similar to readGitConfig's pattern). Update the gitHasPendingChanges function to catch exceptions from execCaptureArgs and return false when git status fails, leaving createEducationalCommit and ensureGitRepository unchanged.react_on_rails/lib/react_on_rails/dev/server_manager.rb (1)
939-944: Consider the marker file cleanup strategy.The marker file at
tmp/react_on_rails/browser_opened_oncepersists across server restarts. This is intentional (to avoid re-opening after the first boot), but developers may want to re-trigger the auto-open without manually deleting the file.Consider documenting the cleanup in the help text or providing a flag like
--reset-browser-marker.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@react_on_rails/lib/react_on_rails/dev/server_manager.rb` around lines 939 - 944, The marker file OPEN_BROWSER_ONCE_MARKER created by mark_browser_opened_once persists and has no opt-in reset; add a new CLI flag (e.g., --reset-browser-marker) parsed where server start options are handled (the server manager/CLI option parsing code that constructs ServerManager), document the new flag in the help text, and when the flag is present call FileUtils.rm_f(OPEN_BROWSER_ONCE_MARKER) (or File.delete with existence check) before starting the server so the auto-open behavior can re-run; ensure any file operations are wrapped in a rescue StandardError to avoid crashing and reference OPEN_BROWSER_ONCE_MARKER and mark_browser_opened_once in the changes.
🤖 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 99-105: Replace the terminal "exit 1" with "return 1" so the
git-history mismatch uses the function return path (allowing the ERR trap and
cleanup_on_error to run and respect KEEP_WORKDIR_ON_FAILURE); specifically
change the branch that checks if "$actual" != "$expected" (which prints
Expected/Actual for $app_dir) to return 1 instead of exiting the shell.
In `@react_on_rails/lib/generators/react_on_rails/base_generator.rb`:
- Around line 622-624: The message returned by home_page_pro_note_for_oss_app is
misleading because it suggests React on Rails Pro can be evaluated "without a
license" while the app is expected to raise on boot if Pro is installed but
unlicensed; update the home_page_pro_note_for_oss_app method to remove the
"without a license" language and instead clearly instruct users to obtain a Pro
license or uninstall Pro (or otherwise state that Pro requires a valid license
and will raise on boot if missing), so the displayed note aligns with the
intended licensed/raise-on-boot behavior.
---
Nitpick comments:
In `@packages/create-react-on-rails-app/scripts/smoke-test-local-gems.sh`:
- Around line 126-176: The test only asserts commit history for APP_TS_DIR and
APP_RSC_JS_DIR, so add expect_git_history checks for the other generated-app
variants (APP_JS_DIR and APP_RSPACK_DIR) to catch commit-message regressions;
update the smoke-test script to call expect_git_history for APP_JS_DIR and
APP_RSPACK_DIR with the expected sequence of commit subjects used by
create-app.ts (matching the patterns used for APP_TS_DIR and APP_RSC_JS_DIR),
and mirror the same addition in the later block referenced around lines 178-204
so all four app variants (APP_JS_DIR, APP_TS_DIR, APP_RSPACK_DIR,
APP_RSC_JS_DIR) have expect_git_history assertions.
In `@packages/create-react-on-rails-app/src/create-app.ts`:
- Around line 179-222: gitHasPendingChanges currently calls
execCaptureArgs('git', ['status', '--porcelain'], appPath) and lets any
exception bubble up; make it resilient by wrapping the execCaptureArgs call in a
try/catch and returning false on error so educational commits remain best-effort
(similar to readGitConfig's pattern). Update the gitHasPendingChanges function
to catch exceptions from execCaptureArgs and return false when git status fails,
leaving createEducationalCommit and ensureGitRepository unchanged.
In `@react_on_rails/lib/react_on_rails/dev/server_manager.rb`:
- Around line 939-944: The marker file OPEN_BROWSER_ONCE_MARKER created by
mark_browser_opened_once persists and has no opt-in reset; add a new CLI flag
(e.g., --reset-browser-marker) parsed where server start options are handled
(the server manager/CLI option parsing code that constructs ServerManager),
document the new flag in the help text, and when the flag is present call
FileUtils.rm_f(OPEN_BROWSER_ONCE_MARKER) (or File.delete with existence check)
before starting the server so the auto-open behavior can re-run; ensure any file
operations are wrapped in a rescue StandardError to avoid crashing and reference
OPEN_BROWSER_ONCE_MARKER and mark_browser_opened_once in the changes.
In `@react_on_rails/spec/react_on_rails/generators/install_generator_spec.rb`:
- Around line 73-96: Add a regression spec that simulates a preexisting
non-commented root route and asserts the generator does not inject or scaffold
over it: reuse the existing context "with --new-app" and
run_generator_test_with_args(%w[--new-app], package_json: true) but with
config/routes.rb prepopulated with an existing root (e.g. 'root to:
"existing#home"'); then assert_file "config/routes.rb" does NOT include 'root
to: "home#index"' and assert_no_file for "app/controllers/home_controller.rb"
and "app/views/home/index.html.erb" (or assert those files are unchanged) to
confirm HomeController and view are not created when a root already exists.
🪄 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: 48e024bc-9080-4c35-8945-d51c1bd68071
📒 Files selected for processing (19)
CHANGELOG.mddocs/oss/getting-started/create-react-on-rails-app.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/src/validators.tspackages/create-react-on-rails-app/tests/create-app.test.tspackages/create-react-on-rails-app/tests/validators.test.tsreact_on_rails/lib/generators/react_on_rails/base_generator.rbreact_on_rails/lib/generators/react_on_rails/generator_messages.rbreact_on_rails/lib/generators/react_on_rails/install_generator.rbreact_on_rails/lib/generators/react_on_rails/templates/base/base/app/controllers/home_controller.rbreact_on_rails/lib/generators/react_on_rails/templates/base/base/app/views/home/index.html.erb.ttreact_on_rails/lib/generators/react_on_rails/templates/base/base/bin/devreact_on_rails/lib/react_on_rails/dev/server_manager.rbreact_on_rails/spec/react_on_rails/dev/server_manager_spec.rbreact_on_rails/spec/react_on_rails/generators/generator_messages_spec.rbreact_on_rails/spec/react_on_rails/generators/install_generator_spec.rb
justin808
force-pushed
the
fix/create-app-onboarding
branch
from
f5911c2 to
3fad485
Compare
| radial-gradient(circle at bottom right, rgba(13, 59, 102, 0.16), transparent 28%), | ||
| var(--page-bg); | ||
| color: var(--ink); | ||
| font-family: "Avenir Next", Avenir, "Segoe UI", sans-serif; |
There was a problem hiding this comment.
The body { margin: 0; ... } rule applies globally and will conflict with any styles the developer adds once they start building beyond the scaffold. Since the entire page content is already wrapped in <div class="ror-demo-page">, consider moving these reset styles onto .ror-demo-page instead:
| font-family: "Avenir Next", Avenir, "Segoe UI", sans-serif; | |
| .ror-demo-page { | |
| margin: 0; | |
| background: | |
| radial-gradient(circle at top left, rgba(239, 131, 84, 0.22), transparent 34%), | |
| radial-gradient(circle at bottom right, rgba(13, 59, 102, 0.16), transparent 28%), | |
| var(--page-bg); | |
| color: var(--ink); | |
| font-family: "Avenir Next", Avenir, "Segoe UI", sans-serif; | |
| } |
Same concern applies to the a and code global rules below — scope them under .ror-demo-page a and .ror-demo-page code.
| class ServerManager | ||
| HELP_FLAGS = ["-h", "--help"].freeze | ||
| TEST_WATCH_MODES = %w[auto full client-only].freeze | ||
| OPEN_BROWSER_WAIT_TIMEOUT = 60 |
There was a problem hiding this comment.
When the server fails to start (DB down, port conflict), this background thread will poll localhost for the full 60 s before giving up — ~120 TCP connect attempts. Since the thread is fire-and-forget this doesn't block the process, but it emits a confusing warn message long after the user has seen the visible error.
Consider a shorter timeout (e.g. 30 s) and/or a comment explaining why 60 s was chosen, so future maintainers know whether it's safe to reduce.
| end | ||
|
|
||
| def browser_auto_open_allowed? | ||
| !ENV.key?("CI") && $stdin.tty? && $stdout.tty? |
There was a problem hiding this comment.
Under Overmind (and some tmux-mux setups), $stdin and $stdout are not attached to a pseudo-TTY even when the developer is running interactively, so this guard will silently skip browser opening. The CI env-var check is the right safety net; the TTY check may be too aggressive in practice.
If the intent is just to avoid running in CI/automation, !ENV.key?("CI") alone is likely sufficient. If keeping the TTY guard, a comment explaining the Overmind/tmux caveat would help users who report "browser never opens".
|
|
||
| private | ||
|
|
||
| def new_app_landing_page_available? |
There was a problem hiding this comment.
This duplicates the same File.foreach + /^\s*root\b/ pattern already in base_generator.rb#preexisting_root_route? and install_generator.rb#new_app_root_route_available?. Three copies means any future fix (e.g. ignoring commented-out root routes inside heredocs, or handling the root "home#index" shorthand) needs to be applied in three places.
Consider extracting a shared helper into GeneratorHelper:
def root_route_declared?
routes_file = File.join(destination_root, "config/routes.rb")
return false unless File.file?(routes_file)
File.foreach(routes_file).any? do |line|
!line.match?(/^\s*#/) && line.match?(/^\s*root\b/)
end
endThen all three callers become a one-liner.
| const DOCS_URL = 'https://reactonrails.com/docs/'; | ||
| const DEFAULT_GIT_AUTHOR_NAME = 'React on Rails Generator'; | ||
| const DEFAULT_GIT_AUTHOR_EMAIL = '[email protected]'; | ||
| const RAILS_GIT_TEMPLATE_RENDERER = ` |
There was a problem hiding this comment.
The ReactOnRailsGitTemplateContext mock reimplements a slice of Rails' internal template API. If a future railties version adds a new method that the .gitignore.tt or .gitattributes.tt templates call, the ERB render will raise a NoMethodError — which is caught and falls back to the hardcoded strings, so it fails silently.
A single comment here listing the Rails methods being mocked and the Rails version they were validated against would make it easier for maintainers to know when this needs updating:
| const RAILS_GIT_TEMPLATE_RENDERER = ` | |
| // Inline Ruby script that renders railties' .gitignore.tt / .gitattributes.tt templates. | |
| // Mocks the subset of Rails' template binding used by these two files as of Rails 7.2. | |
| // If a future Rails version adds new template methods, this will fall back to the | |
| // RAILS_GITIGNORE_FALLBACK / RAILS_GITATTRIBUTES_FALLBACK constants below. | |
| const RAILS_GIT_TEMPLATE_RENDERER = ` |
|
Following up on #2849 (comment): I moved the three maintainability-focused discuss items into follow-up issue #2890 so they can be handled outside this merge path: Skipping the remaining items for this PR:
No code changes needed for this comment in PR #2849. |
…e-repo * origin/main: Improve create-react-on-rails-app fresh app onboarding (#2849) Add Docker deployment guide for Kamal, Kubernetes, and Control Plane (#2837) Reframe RSC migration docs around React on Rails patterns (#2661) Enforce secure renderer password defaults for production-like envs (#2829)
…create-app tests The create-app tests used toHaveBeenNthCalledWith and counted mockImplementationOnce chains that coupled to the exact interleaving of educational git commits. Adding any intermediate execLiveArgs call broke 13 of 78 tests even when behavior was identical. Replace positional ordinals with toHaveBeenCalledWith (argument matching without position), replace mockImplementationOnce chains with command-matching mockImplementation, and replace toHaveBeenCalledTimes with a filtered stepCalls() helper that counts non-git commands only. Addresses item 1 of #2888 (deferred from #2849 review). Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
## Summary - Replace `toHaveBeenNthCalledWith` ordinals with `toHaveBeenCalledWith` (semantic argument matching) - Replace `mockImplementationOnce` chains with command-matching `mockImplementation` - Replace `toHaveBeenCalledTimes` with `stepCallSummaries()` — a filtered sequence assertion on non-git commands that verifies ordering, count, and argument correctness in one check - Add `stepCallSummaries()` helper (mirrors the existing `gitCommitSubjects()` pattern for commit ordering) ## Context PR #2849 added educational git commits to the `create-react-on-rails-app` scaffold flow, interleaving `git add` + `git commit` calls between each real step. The existing tests were updated to use new ordinal positions (1→4→7→10) and counts (9, 12), but a reviewer flagged this as brittle and it was deferred as item 1 of #2888. The brittleness was verified: adding a single `execLiveArgs` call to `create-app.ts` broke 13 of 78 tests — 7 from shifted ordinals, 5 from `mockImplementationOnce` chain off-by-one, and 1 from a count mismatch. ## Approach The test file already had `gitCommitSubjects().toEqual([...])` which verifies commit ordering through `mock.calls` filtering — the [standard Jest community pattern](jestjs/jest#4402) for asserting call order on a subset of calls. This PR extends that pattern to step commands via `stepCallSummaries()`, which filters out educational git operations and asserts on the exact sequence of real steps: ```typescript expect(stepCallSummaries()).toEqual([ 'rails new', 'bundle add react_on_rails', 'bundle add react_on_rails_pro', 'bundle generate', ]); ``` This provides ordering verification (via `toEqual` on an ordered array), count verification (exact array length), and resilience to educational-commit interleaving — without the positional coupling of the original ordinal assertions. Partially addresses #2888. ## Test plan - [x] All 78 existing tests pass - [x] Adding an extra `git` command to `create-app.ts` → 0 test failures (correctly tolerant of git interleaving) - [x] Adding an extra non-git command to `create-app.ts` → `stepCallSummaries()` catches the sequence mismatch - [x] Inserting a command at the wrong position → `stepCallSummaries()` catches the ordering violation - [x] Pre-commit hooks pass (prettier, eslint, trailing-newlines) 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Improved test reliability and maintainability for the app-creation flow by adding helpers to summarize and verify ordered setup steps, shifting assertions from strict call-count/order to presence and expected step sequences, and consolidating failure-path mocks for clearer, more robust error simulations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
Server manager browser-open improvements: - Refactor Thread.new block: remove non-idiomatic `next`, remove `report_on_exception = false` that suppressed genuine crashes - Add WSL browser support: detect WSL_DISTRO_NAME/WSLENV and try wslview/wsl-open before falling back to xdg-open - Warn user when no browser launcher is found (with WSL-specific hint) - Let explicit --open-browser bypass TTY/CI guard since the developer deliberately requested it; auto-opens still respect the guard - Resolve marker-file path lazily at call time instead of at constant-definition time to avoid wrong path when required before chdir into Rails root - Narrow rescued exceptions in http_get_localhost from broad StandardError to specific socket/connection error classes Generator fix: - Update misleading Pro note that said Pro works "without a license" (Pro raises on boot without one) Closes #2880 Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Server manager browser-open improvements: - Refactor Thread.new block: remove non-idiomatic `next`, remove `report_on_exception = false` that suppressed genuine crashes - Add WSL browser support: detect WSL_DISTRO_NAME/WSLENV and try wslview/wsl-open before falling back to xdg-open - Warn user when no browser launcher is found (with WSL-specific hint) - Let explicit --open-browser bypass TTY/CI guard since the developer deliberately requested it; auto-opens still respect the guard - Resolve marker-file path lazily at call time instead of at constant-definition time to avoid wrong path when required before chdir into Rails root - Narrow rescued exceptions in http_get_localhost from broad StandardError to specific socket/connection error classes Generator fix: - Update misleading Pro note that said Pro works "without a license" (Pro raises on boot without one) Closes #2880 Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
## Summary - Replace `toHaveBeenNthCalledWith` ordinals with `toHaveBeenCalledWith` (semantic argument matching) - Replace `mockImplementationOnce` chains with command-matching `mockImplementation` - Replace `toHaveBeenCalledTimes` with `stepCallSummaries()` — a filtered sequence assertion on non-git commands that verifies ordering, count, and argument correctness in one check - Add `stepCallSummaries()` helper (mirrors the existing `gitCommitSubjects()` pattern for commit ordering) ## Context PR #2849 added educational git commits to the `create-react-on-rails-app` scaffold flow, interleaving `git add` + `git commit` calls between each real step. The existing tests were updated to use new ordinal positions (1→4→7→10) and counts (9, 12), but a reviewer flagged this as brittle and it was deferred as item 1 of #2888. The brittleness was verified: adding a single `execLiveArgs` call to `create-app.ts` broke 13 of 78 tests — 7 from shifted ordinals, 5 from `mockImplementationOnce` chain off-by-one, and 1 from a count mismatch. ## Approach The test file already had `gitCommitSubjects().toEqual([...])` which verifies commit ordering through `mock.calls` filtering — the [standard Jest community pattern](jestjs/jest#4402) for asserting call order on a subset of calls. This PR extends that pattern to step commands via `stepCallSummaries()`, which filters out educational git operations and asserts on the exact sequence of real steps: ```typescript expect(stepCallSummaries()).toEqual([ 'rails new', 'bundle add react_on_rails', 'bundle add react_on_rails_pro', 'bundle generate', ]); ``` This provides ordering verification (via `toEqual` on an ordered array), count verification (exact array length), and resilience to educational-commit interleaving — without the positional coupling of the original ordinal assertions. Partially addresses #2888. ## Test plan - [x] All 78 existing tests pass - [x] Adding an extra `git` command to `create-app.ts` → 0 test failures (correctly tolerant of git interleaving) - [x] Adding an extra non-git command to `create-app.ts` → `stepCallSummaries()` catches the sequence mismatch - [x] Inserting a command at the wrong position → `stepCallSummaries()` catches the ordering violation - [x] Pre-commit hooks pass (prettier, eslint, trailing-newlines) 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Improved test reliability and maintainability for the app-creation flow by adding helpers to summarize and verify ordered setup steps, shifting assertions from strict call-count/order to presence and expected step sequences, and consolidating failure-path mocks for clearer, more robust error simulations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
## Summary - Replace `toHaveBeenNthCalledWith` ordinals with `toHaveBeenCalledWith` (semantic argument matching) - Replace `mockImplementationOnce` chains with command-matching `mockImplementation` - Replace `toHaveBeenCalledTimes` with `stepCallSummaries()` — a filtered sequence assertion on non-git commands that verifies ordering, count, and argument correctness in one check - Add `stepCallSummaries()` helper (mirrors the existing `gitCommitSubjects()` pattern for commit ordering) ## Context PR #2849 added educational git commits to the `create-react-on-rails-app` scaffold flow, interleaving `git add` + `git commit` calls between each real step. The existing tests were updated to use new ordinal positions (1→4→7→10) and counts (9, 12), but a reviewer flagged this as brittle and it was deferred as item 1 of #2888. The brittleness was verified: adding a single `execLiveArgs` call to `create-app.ts` broke 13 of 78 tests — 7 from shifted ordinals, 5 from `mockImplementationOnce` chain off-by-one, and 1 from a count mismatch. ## Approach The test file already had `gitCommitSubjects().toEqual([...])` which verifies commit ordering through `mock.calls` filtering — the [standard Jest community pattern](jestjs/jest#4402) for asserting call order on a subset of calls. This PR extends that pattern to step commands via `stepCallSummaries()`, which filters out educational git operations and asserts on the exact sequence of real steps: ```typescript expect(stepCallSummaries()).toEqual([ 'rails new', 'bundle add react_on_rails', 'bundle add react_on_rails_pro', 'bundle generate', ]); ``` This provides ordering verification (via `toEqual` on an ordered array), count verification (exact array length), and resilience to educational-commit interleaving — without the positional coupling of the original ordinal assertions. Partially addresses #2888. ## Test plan - [x] All 78 existing tests pass - [x] Adding an extra `git` command to `create-app.ts` → 0 test failures (correctly tolerant of git interleaving) - [x] Adding an extra non-git command to `create-app.ts` → `stepCallSummaries()` catches the sequence mismatch - [x] Inserting a command at the wrong position → `stepCallSummaries()` catches the ordering violation - [x] Pre-commit hooks pass (prettier, eslint, trailing-newlines) 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Improved test reliability and maintainability for the app-creation flow by adding helpers to summarize and verify ordered setup steps, shifting assertions from strict call-count/order to presence and expected step sequences, and consolidating failure-path mocks for clearer, more robust error simulations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
Server manager browser-open improvements: - Refactor Thread.new block: remove non-idiomatic `next`, remove `report_on_exception = false` that suppressed genuine crashes - Add WSL browser support: detect WSL_DISTRO_NAME/WSLENV and try wslview/wsl-open before falling back to xdg-open - Warn user when no browser launcher is found (with WSL-specific hint) - Let explicit --open-browser bypass TTY/CI guard since the developer deliberately requested it; auto-opens still respect the guard - Resolve marker-file path lazily at call time instead of at constant-definition time to avoid wrong path when required before chdir into Rails root - Narrow rescued exceptions in http_get_localhost from broad StandardError to specific socket/connection error classes Generator fix: - Update misleading Pro note that said Pro works "without a license" (Pro raises on boot without one) Closes #2880 Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
## Summary - Refactor server manager browser-open: remove non-idiomatic `next` in Thread.new, remove `report_on_exception = false` that suppressed genuine crashes, add WSL browser support (wslview/wsl-open), let explicit `--open-browser` bypass TTY guard, resolve marker path lazily, narrow rescued socket exceptions - Fix misleading Pro note that said Pro works "without a license" ## Details ### Server manager (`server_manager.rb`) - **Thread.new refactor**: Replaced `next` (non-idiomatic in Thread.new) with standard conditionals; removed `report_on_exception = false` so genuine crashes surface in development - **WSL support**: Detects WSL via `WSL_DISTRO_NAME`/`WSLENV` env vars and tries `wslview`/`wsl-open` before `xdg-open` - **Platform diagnostic**: Warns user when no browser launcher is found (with WSL-specific install hint) - **Explicit vs auto-open TTY gating**: `--open-browser` (deliberate user request) now bypasses TTY/CI guard; `--open-browser-once` (auto-open) still respects it - **Lazy marker path**: Replaced `OPEN_BROWSER_ONCE_MARKER` constant (resolved at class-load time) with `open_browser_once_marker` method (resolved at call time) to avoid wrong path when file is required before chdir into Rails root - **Narrowed exception handling**: `http_get_localhost` now rescues specific socket/connection errors instead of broad `StandardError` ### Generator (`base_generator.rb`) - Updated `home_page_pro_note_for_oss_app` to not claim Pro works "without a license" (Pro raises on boot when unlicensed) Closes #2880 ## Test plan - [x] `bundle exec rspec spec/react_on_rails/dev/server_manager_spec.rb` — 93 examples, 0 failures - [x] `bundle exec rspec spec/react_on_rails/generators/install_generator_spec.rb:73` — passes - [x] `bundle exec rubocop` on all changed files — no offenses - [ ] Verify WSL browser detection in WSL environment - [ ] Verify `--open-browser` works in non-TTY context (e.g., Docker) 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Medium risk because it changes development process management and SSR webpack configuration, which can affect local workflows and server-bundle debugging/stack traces across platforms. > > **Overview** > **Stabilizes development ergonomics across environments.** `ServerManager` browser auto-open was refactored to distinguish explicit `--open-browser` from guarded auto-open (`--open-browser-once`), add WSL launcher support (`wslview`/`wsl-open`), improve failure warnings, lazily resolve the once-marker path, and narrow localhost polling exception handling. > > **Avoids a webpack 5.106+ SSR regression.** Server webpack configs switch away from `devtool = 'eval'` to `cheap-module-source-map` in development (and `false` in production) to prevent ESM default-export issues and improve SSR stack trace mapping. > > Also updates generator messaging to remove the claim that Pro can be evaluated without a license, fixes generator `chmod` to use `destination_root`, adds `.playwright-cli/` to `.gitignore`, and adds a Jest assertion ensuring `scriptSanitizedVal` keeps a stable function name for webpack compatibility. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit bd84af3. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Fixed server-side rendering (SSR) error stack traces to accurately map line numbers by updating source map configuration. * Improved browser auto-open functionality on WSL systems with better launcher detection and fallback handling. * **Documentation** * Updated Pro licensing guidance to clarify users must review upgrade documentation and obtain proper licenses before enabling Pro features. * **Tests** * Added test coverage for function naming stability and browser auto-open behavior across different environments. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
…ages * origin/main: Fix initial page startup race for late-loading client bundles (#3151) chore: apply prettier formatting to tracked docs files (#3153) docs: comprehensive RSC API documentation and registration consolidation (#3140) Split rspec-package-tests into parallel generator/unit shards (#3134) fix: add concurrency groups to long-running CI workflows (#3133) refactor: add RenderRequest, JsCodeBuilder, and RenderingStrategy abstractions (#3094) fix: address deferred review items from PR #2849 (#3093) Add complimentary OSS license policy for React on Rails Pro (#3123) fix: centralize CI docs-only detection and add CLI flag validation (#3091) refactor: replace stub-throw + Object.assign with capability-based composition (#3096) Enhance address-review with parallel fixes, self-review, and Greptile verification (#3121) fix: Doctor no longer fails custom projects for missing bin/dev (#3117) fix: cap webpack <5.106.0 to prevent ExecJS SSR breakage (#3095) Add Rspack + RSC compatibility tests and documentation (#1828) (#3120) Add error scenarios hub and test pages (#2497) docs: document polyfill requirements for web-targeted server bundles (#3092) docs: RSC integration pitfalls from tutorial app (#3087) docs: fix render function/helper API documentation (#3088) Doctor: accept TS/TSX server bundle suffixes (#3111) feat: add CI guard requiring sidebar updates when adding docs (#3089)
## Summary - Refactor server manager browser-open: remove non-idiomatic `next` in Thread.new, remove `report_on_exception = false` that suppressed genuine crashes, add WSL browser support (wslview/wsl-open), let explicit `--open-browser` bypass TTY guard, resolve marker path lazily, narrow rescued socket exceptions - Fix misleading Pro note that said Pro works "without a license" ## Details ### Server manager (`server_manager.rb`) - **Thread.new refactor**: Replaced `next` (non-idiomatic in Thread.new) with standard conditionals; removed `report_on_exception = false` so genuine crashes surface in development - **WSL support**: Detects WSL via `WSL_DISTRO_NAME`/`WSLENV` env vars and tries `wslview`/`wsl-open` before `xdg-open` - **Platform diagnostic**: Warns user when no browser launcher is found (with WSL-specific install hint) - **Explicit vs auto-open TTY gating**: `--open-browser` (deliberate user request) now bypasses TTY/CI guard; `--open-browser-once` (auto-open) still respects it - **Lazy marker path**: Replaced `OPEN_BROWSER_ONCE_MARKER` constant (resolved at class-load time) with `open_browser_once_marker` method (resolved at call time) to avoid wrong path when file is required before chdir into Rails root - **Narrowed exception handling**: `http_get_localhost` now rescues specific socket/connection errors instead of broad `StandardError` ### Generator (`base_generator.rb`) - Updated `home_page_pro_note_for_oss_app` to not claim Pro works "without a license" (Pro raises on boot when unlicensed) Closes #2880 ## Test plan - [x] `bundle exec rspec spec/react_on_rails/dev/server_manager_spec.rb` — 93 examples, 0 failures - [x] `bundle exec rspec spec/react_on_rails/generators/install_generator_spec.rb:73` — passes - [x] `bundle exec rubocop` on all changed files — no offenses - [ ] Verify WSL browser detection in WSL environment - [ ] Verify `--open-browser` works in non-TTY context (e.g., Docker) 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Medium risk because it changes development process management and SSR webpack configuration, which can affect local workflows and server-bundle debugging/stack traces across platforms. > > **Overview** > **Stabilizes development ergonomics across environments.** `ServerManager` browser auto-open was refactored to distinguish explicit `--open-browser` from guarded auto-open (`--open-browser-once`), add WSL launcher support (`wslview`/`wsl-open`), improve failure warnings, lazily resolve the once-marker path, and narrow localhost polling exception handling. > > **Avoids a webpack 5.106+ SSR regression.** Server webpack configs switch away from `devtool = 'eval'` to `cheap-module-source-map` in development (and `false` in production) to prevent ESM default-export issues and improve SSR stack trace mapping. > > Also updates generator messaging to remove the claim that Pro can be evaluated without a license, fixes generator `chmod` to use `destination_root`, adds `.playwright-cli/` to `.gitignore`, and adds a Jest assertion ensuring `scriptSanitizedVal` keeps a stable function name for webpack compatibility. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit bd84af3. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Fixed server-side rendering (SSR) error stack traces to accurately map line numbers by updating source map configuration. * Improved browser auto-open functionality on WSL systems with better launcher detection and fallback handling. * **Documentation** * Updated Pro licensing guidance to clarify users must review upgrade documentation and obtain proper licenses before enabling Pro features. * **Tests** * Added test coverage for function naming stability and browser auto-open behavior across different environments. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
3 participants
Summary
bin/devcreate-react-on-rails-appto validategitand create educational commits for each scaffold stepTesting
Note
Medium Risk
Changes the app scaffolding workflow across the Node CLI and Ruby generators (new root route/landing page, git automation, and new
bin/devbrowser-opening behavior), which could affect fresh-app creation flows and developer startup scripts across platforms.Overview
Freshly scaffolded apps now start at a generated root landing page (
/) with links to the SSR/RSC demo routes, key files, and curated OSS vs Pro/RSC resources. This is enabled via a new hidden--new-appgenerator mode that injectsroot to: "home#index"(when safe), addsHomeController+home/indextemplate, and updates demo views to include richer onboarding content and cross-links.bin/devcan now auto-open the browser once and supports new flags (--open-browser,--open-browser-once,--no-open-browser), implemented inReactOnRails::Dev::ServerManagerwith a one-time marker file and route/URL normalization. The generator configures fresh apps to default to/and enable one-time open.create-react-on-rails-appnow requiresgitand records educational commits for each scaffold phase (Rails app creation, gem adds, generator run, pnpm normalization), usingrails new --skip-gitplus restored Rails.gitignore/.gitattributestemplates for clean commits and disabling GPG signing for automation. Smoke tests, docs, and changelog are updated to cover--pro, the landing page, browser auto-open, and the new commit history.Written by Cursor Bugbot for commit 81e287e. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
New Features
Documentation