Walkthrough

Trimmed app names are now validated (must start with a letter), Rails version validation requires 7.0+, tests extended for success and failure paths, and related docs and package.json formatting were updated.

Changes

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• Fix create-react-on-rails-app follow-ups for #2383 #2577: Mirrors the create-react-on-rails-app validation updates (trim/name-start regex and Rails 7.0+ check) and associated tests/docs.

Poem

🐰 I trim the name, I check the start,
I hop for letters, no leading dash or chart,
Rails at seven, standing tall and bright,
Tests hop in, asserting day and night,
A tidy generator — I nibble with delight!

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Overall the direction is good. Tighter app name validation, composable errors instead of process.exit, cross-platform clean, and the new Rails 7.0+ prerequisite check all improve the package. Test coverage additions are appreciated. Three things worth addressing before merge: (1) execLive is exported but never called anywhere in the codebase - dead code; either remove it or document the use-case, and note why shell:true is needed over the tokenised execLiveArgs. (2) createStepError discards the original stack trace - prefer new Error(message, { cause: error }) (Node 16.9+) so the full chain stays accessible. (3) MIN_RAILS_MINOR and MIN_RUBY_MINOR are both 0, making the minor < MIN_*_MINOR branch always-false dead code; harmless now but could mislead a future maintainer who bumps the minimum. Inline comments posted for all three.

PR Review: Fix create-react-on-rails-app follow-ups

Overall this is a clean, focused PR. The refactors are well-motivated and the test coverage additions are good. A few issues worth addressing:

1. execLive is exported but never used — utils.ts adds execLive(command, cwd) with shell: true, but it is never called. A dead export with shell: true is a latent security risk. Remove it now and add it in the PR that actually needs it, or at minimum leave it unexported until it has a caller.

2. createStepError drops the original stack trace — Constructing a brand-new Error with only the message string discards the original call stack. Node 16.9+ supports the cause option: new Error(message, { cause: error }). This preserves the full stack while still surfacing a user-friendly message.

3. Missing happy-path test for createApp — All three new createApp tests cover failure branches. There is no test verifying that when all execLiveArgs calls succeed the function completes without throwing. A simple fourth case would catch regressions in the success path.

4. Minor: misleading error message for names with leading whitespace — validateAppName(' myapp') returns 'must start with a letter' because the un-trimmed string is compared against the regex after the whitespace guard. Trimming before the regex (or adjusting the guard) would produce a more accurate error.

Positives:

• Replacing process.exit(1) inside helper functions with thrown errors is the right design; index.ts now owns the exit boundary cleanly.
• buildGeneratorArgs refactor to spread syntax is more concise.
• Rails 7.0+ version check is well-implemented with good test coverage.
• Cross-platform clean script via node -e is a nice improvement.

Greptile Summary

This PR is a follow-up to #2375, delivering a set of polish and correctness improvements to the create-react-on-rails-app CLI package. The changes are clean, well-tested, and fit naturally into the existing codebase patterns.

Key changes:

• Stricter app name validation: Regex updated from /^[a-zA-Z0-9_-]+$/ to /^[a-zA-Z][a-zA-Z0-9_-]*$/, ensuring names must start with a letter — consistent with Ruby/Rails directory naming requirements.
• Rails 7.0+ prerequisite check: validateRails() now parses and compares the rails --version output, rejecting older versions with a clear error message. The version comparison logic is correct and mirrors the existing Ruby validator pattern.
• Composable error propagation: createApp no longer calls process.exit(1) directly on step failures; instead it throws errors via a createStepError helper. These bubble up to the top-level action() catch handler in index.ts, which logs and exits — a much cleaner design.
• Cross-platform clean script: rm -rf ./lib replaced with a node -e equivalent using fs.rmSync.
• Test coverage expanded: New tests for rejecting names starting with digits/hyphens/underscores, each createApp failure step, Rails version validation (6.1 rejection, unparseable version), and the validateAll success path.

Two minor improvements recommended:

1. The newly exported execLive function in utils.ts is unused dead code and should be removed or deferred to when it's actually needed.
2. Add a happy-path test case for createApp to verify successful completion when all steps pass.

Confidence Score: 4/5

• This PR is safe to merge. All functional changes are correct; the two flagged items are minor improvements that don't affect correctness.
• The functional changes are solid: app name validation correctly enforces leading-letter requirement, Rails 7.0+ version check is properly implemented, and error handling refactoring is clean. Test coverage is comprehensive for the main functionality. The only items flagged are: (1) unused execLive export in utils.ts—dead code that should be removed or deferred, and (2) missing happy-path test for createApp—a testing completeness suggestion. Neither impacts functionality or introduces bugs.
• packages/create-react-on-rails-app/src/utils.ts (remove unused execLive export) and packages/create-react-on-rails-app/tests/create-app.test.ts (add happy-path test)

Sequence Diagram

sequenceDiagram
    participant CLI as index.ts (CLI entry)
    participant V as validators.ts
    participant CA as create-app.ts
    participant OS as OS / Shell

    CLI->>CA: validateAppName(appName)
    note over CA: Regex: /^[a-zA-Z][a-zA-Z0-9_-]*$/
    CA-->>CLI: { success, error? }

    CLI->>V: validateAll(packageManager)
    V->>V: validateNode()
    V->>V: validateRuby() — ruby --version, ≥3.0
    V->>V: validateRails() — rails --version, ≥7.0 (NEW)
    V->>V: validatePackageManager()
    V-->>CLI: { allValid, results }

    alt allValid
        CLI->>CA: createApp(appName, options)
        CA->>OS: rails new appName
        OS-->>CA: exit 0 (or throws)
        CA->>OS: bundle add react_on_rails (in appPath)
        OS-->>CA: exit 0 (or throws)
        CA->>OS: bundle exec rails generate react_on_rails:install (in appPath)
        OS-->>CA: exit 0 (or throws)
        CA-->>CLI: (returns)
    else not allValid
        CLI->>CLI: logError + process.exit(1)
    end

    note over CLI: Errors thrown by createApp bubble up<br/>to action() catch → logError + exit(1)

_{Last reviewed commit: cdd0027}

size-limit report 📦

Addressed in commit e058edc:\n\n- removed unused exported \n- preserved wrapped error context by setting \n- simplified the Rails minimum-version check to avoid dead minor-version logic\n- normalized app name handling (trim before validation and use normalized name in orchestration)\n- added a happy-path \ test plus additional validation assertions\n- added an explicit Rails 7.0.x acceptance test\n- updated docs/changelog/proposal wording for consistency\n\nI intentionally kept full Rails \ parity out of this follow-up so this stays scoped to #2383; \Usage:
rails new APP_PATH [options]

Options:
[--skip-namespace] # Skip namespace (affects only isolated engines)
# Default: false
[--skip-collision-check] # Skip collision check
# Default: false
-r, [--ruby=PATH] # Path to the Ruby binary of your choice
# Default: /Users/justin/.local/share/mise/installs/ruby/3.4.3/bin/ruby
-n, [--name=NAME] # Name of the app
-m, [--template=TEMPLATE] # Path to some application template (can be a filesystem path or URL)
-d, [--database=DATABASE] # Preconfigure for selected database
# Default: sqlite3
# Possible values: mysql, trilogy, postgresql, sqlite3, mariadb-mysql, mariadb-trilogy
-G, [--skip-git] # Skip git init, .gitignore and .gitattributes
[--skip-docker] # Skip Dockerfile, .dockerignore and bin/docker-entrypoint
[--skip-keeps] # Skip source control .keep files
-M, [--skip-action-mailer] # Skip Action Mailer files
[--skip-action-mailbox] # Skip Action Mailbox gem
[--skip-action-text] # Skip Action Text gem
-O, [--skip-active-record] # Skip Active Record files
[--skip-active-job] # Skip Active Job
[--skip-active-storage] # Skip Active Storage files
-C, [--skip-action-cable] # Skip Action Cable files
-A, [--skip-asset-pipeline] # Skip the asset pipeline setup
-J, --skip-js, [--skip-javascript] # Skip JavaScript files
[--skip-hotwire] # Skip Hotwire integration
[--skip-jbuilder] # Skip jbuilder gem
-T, [--skip-test] # Skip test files
[--skip-system-test] # Skip system test files
[--skip-bootsnap] # Skip bootsnap gem
[--skip-dev-gems] # Skip development gems (e.g., web-console)
[--skip-thruster] # Skip Thruster setup
[--skip-rubocop] # Skip RuboCop setup
[--skip-brakeman] # Skip brakeman setup
[--skip-bundler-audit] # Skip bundler-audit setup
[--skip-ci] # Skip GitHub CI files
[--skip-kamal] # Skip Kamal setup
[--skip-solid] # Skip Solid Cache, Queue, and Cable setup
[--dev], [--no-dev], [--skip-dev] # Set up the application with Gemfile pointing to your Rails checkout
[--devcontainer], [--no-devcontainer], [--skip-devcontainer] # Generate devcontainer files
[--edge], [--no-edge], [--skip-edge] # Set up the application with a Gemfile pointing to the 8-1-stable branch on the Rails repository
--master, [--main], [--no-main], [--skip-main] # Set up the application with Gemfile pointing to Rails repository main branch
[--rc=RC] # Path to file containing extra configuration options for rails command
[--no-rc] # Skip loading of extra configuration options from .railsrc file
[--api], [--no-api], [--skip-api] # Preconfigure smaller stack for API only apps
# Default: false
[--minimal], [--no-minimal], [--skip-minimal] # Preconfigure a minimal rails app
-j, --js, [--javascript=JAVASCRIPT] # Choose JavaScript approach
# Default: importmap
# Possible values: importmap, bun, webpack, esbuild, rollup
-c, [--css=CSS] # Choose CSS processor. Check https://github.com/rails/cssbundling-rails for more options
# Possible values: tailwind, bootstrap, bulma, postcss, sass
-B, [--skip-bundle] # Don't run bundle install
[--skip-decrypted-diffs] # Don't configure git to show decrypted diffs of encrypted credentials

Runtime options:
-f, [--force] # Overwrite files that already exist
-p, [--pretend], [--no-pretend], [--skip-pretend] # Run but do not make any changes
-q, [--quiet], [--no-quiet], [--skip-quiet] # Suppress status output
-s, [--skip], [--no-skip], [--skip-skip] # Skip files that already exist

Rails options:
-h, [--help], [--no-help], [--skip-help] # Show this help message and quit
-v, [--version], [--no-version], [--skip-version] # Show Rails version number and quit

Description:
The rails new command creates a new Rails application with a default
directory structure and configuration at the path you specify.

You can specify extra command-line arguments to be used every time
`rails new` runs in the .railsrc configuration file in your home directory,
or in $XDG_CONFIG_HOME/rails/railsrc if XDG_CONFIG_HOME is set.

Note that the arguments specified in the .railsrc file don't affect the
default values shown above in this help message.

You can specify which version to use when creating a new rails application 
using `rails _<version>_ new`.

Examples:
rails new ~/Code/Ruby/weblog

This generates a new Rails app in ~/Code/Ruby/weblog.

`rails _<version>_ new weblog`

This generates a new Rails app with the provided version in ./weblog.

`rails new weblog --api`

This generates a new Rails app in API mode in ./weblog.

`rails new weblog --skip-action-mailer`

This generates a new Rails app without Action Mailer in ./weblog.
Any part of Rails can be skipped during app generation. remains the authoritative guard for reserved-name collisions.

Addressed in commit e058edc:

• removed unused exported execLive
• preserved wrapped error context by setting wrappedError.cause
• simplified the Rails minimum-version check to avoid dead minor-version logic
• normalized app name handling (trim before validation and use normalized name in orchestration)
• added a happy-path createApp test plus additional validation assertions
• added an explicit Rails 7.0.x acceptance test
• updated docs/changelog/proposal wording for consistency

I intentionally kept full Rails AppName valid_const parity out of this follow-up so this stays scoped to #2383; rails new remains the authoritative guard for reserved-name collisions.

Review Summary

The changes are generally clean and well-tested. A few minor issues worth addressing:

Bug-free / no security concerns — the app name validation, Rails 7.0+ enforcement, and trimming logic are all correct.

Nits / minor issues (inline comments posted)

1. Duplicate test (tests/create-app.test.ts): 'rejects names that start with a number' is a near-duplicate of the existing 'rejects names starting with a digit' test.

2. Redundant empty-string check (src/create-app.ts:89): !name is always false when trimmedName === '' covers it — can simplify to if (!trimmedName).

3. Misleading error message (src/create-app.ts:95-98): The single regex-failure message says "must start with a letter" but is also returned for names like 'my app' that start with a letter but contain a space. The test asserts toContain('must start with a letter') for that case, which only passes incidentally.

4. Double-trim (src/index.ts:13): appName.trim() is called here and inside validateAppName. One of the two is redundant — worth clarifying the API contract.

5. Spurious CHANGELOG line (CHANGELOG.md:28): "Changes since the last non-beta release." was accidentally inserted and doesn't match the established format.

PR Review: Fix create-react-on-rails-app follow-ups

Overall these are clean, focused improvements. Trimming behavior, stricter regex enforcement, and Rails 7.0+ validation are all solid. Test coverage expansion is good. A few minor issues below.

1. Redundant null-check in validateAppName

name.trim() is called before the falsy guard on line 89 of create-app.ts. This means name cannot be null/undefined at that point. The !name branch only guards against name === '', which is already covered by trimmedName === ''. The !name check is redundant for any valid TypeScript caller and can be removed.

2. Duplicate digit-start test case

create-app.test.ts now has two nearly identical tests: one testing 1app and one testing 123app. Both hit the same regex branch. One should be removed.

3. Missing test for leading/trailing whitespace trimming

validateAppName now trims input internally, but no test exercises this path directly. A test passing a name with leading/trailing spaces (e.g., ' myapp ') and expecting success: true would confirm the trim behavior and guard against accidental regression.

Overall this is a clean, focused set of follow-up fixes. The validation tightening and test additions are well done. One functional issue and a couple of minor observations below.

Bug: trimmed name not propagated to createApp

The main issue is in validateAppName: it validates against trimmedName internally but returns success:true without the trimmed value. The caller in index.ts then passes the original appName (untrimmed) to createApp (see inline comment).

Before this PR, a whitespace-padded input would have been rejected by the regex since the raw string starts with a space. After this PR it silently passes validation, but createApp still receives the space-padded string and would call rails new with it. Fix: return the trimmed name from validateAppName (e.g. return { success: true, name: trimmedName }), or trim appName in index.ts before both calls.

Minor observations

Two tests for Rails 6.1: validators.test.ts now has both 'returns invalid for Rails 6.1' (new, 6.1.7.10) and 'returns invalid for Rails 6.x' (existing, 6.1.7). The new one is better since it asserts message wording, but they cover the same code path and are largely redundant.

Redundant null guard: the first if (!name) handles null/undefined/empty-string, and the second if (!trimmedName) handles whitespace-only. The original single expression was equivalent and more concise.

What is good

Stricter regex applied to trimmedName is the right call. Test assertions on error message wording are a solid improvement over bare false checks. The new Rails 7.0.x test with a four-part version string (7.0.8.7) adds genuine coverage for a format the previous tests missed. Docs and proposal cleanup are clear and accurate.