Walkthrough

Adds pretend-mode guards and option propagation to the install and base generators so shell commands, raw File.chmod calls, and several install-related creation steps are skipped and report pretend-status messages when --pretend is used; tests and changelog updated to reflect the dry-run semantics.

Changes

Sequence Diagram

sequenceDiagram
    participant User
    participant Generator
    participant ShellCmd
    participant FileSystem

    User->>Generator: run generator (rails g react_on_rails:install [--pretend])
    
    rect rgba(100,150,255,0.5)
    Note over Generator: Normal mode (no --pretend)
    Generator->>ShellCmd: run bundle add / bundle install / rails shakapacker:install
    ShellCmd->>FileSystem: modify Gemfile, install gems, create shakapacker files
    Generator->>FileSystem: File.chmod bin scripts
    Generator-->>User: report installation complete
    end

    rect rgba(255,150,100,0.5)
    Note over Generator: Pretend mode (--pretend)
    Generator->>Generator: check options[:pretend]
    Generator-->>ShellCmd: skip shell commands (pretend)
    Generator-->>FileSystem: skip file writes & File.chmod (pretend)
    Generator-->>User: emit pretend-status messages / show preview
    end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

• #2422 — Tests/changes touch ensure_shakapacker_installed; this PR's pretend guards relate to install flow and may overlap with post-install handling discussed in that issue.

Poem

🐰 I hopped through lines with gentle paws,
Skipping installs and chmods because,
When --pretend whispers soft and light,
I show the plan and do not bite.
A safe dry-run — hop, preview, 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.}

Greptile Summary

This PR fixes two --pretend (dry-run) mode bugs in InstallGenerator. It adds an early return inside ensure_shakapacker_installed when options[:pretend] is true so that no real bundle add/bundle install/rails shakapacker:install commands are executed, and it guards the raw File.chmod call in add_bin_scripts with unless options[:pretend] to prevent an Errno::ENOENT crash when the bin files were never actually written. Two focused regression specs are added to cover both paths.

• ensure_shakapacker_installed: Early return with a say_status notification when --pretend is active, preventing all four real side-effect methods from being called.
• add_bin_scripts: File.chmod is now skipped in pretend mode; the preceding directory and gsub_file calls are Thor generator actions that already natively respect --pretend.
• Tests: New describe "--pretend mode behavior" block with unit-level specs for both changed methods, using appropriate stubs and send to access private methods.

Confidence Score: 5/5

• This PR is safe to merge — changes are minimal, targeted, and do not alter any non-pretend-mode behavior.
• Both fixes are single-line or short guard additions that only activate when options[:pretend] is true, leaving the normal execution path completely unchanged. The regression tests correctly exercise both code paths. No new dependencies or public API changes are introduced.
• No files require special attention.

Important Files Changed

Sequence Diagram

sequenceDiagram
    participant User
    participant InstallGenerator
    participant FileSystem as FileSystem / Bundler

    User->>InstallGenerator: rails g react_on_rails:install --pretend

    InstallGenerator->>InstallGenerator: ensure_shakapacker_installed()
    InstallGenerator->>InstallGenerator: shakapacker_configured? → false
    InstallGenerator->>InstallGenerator: options[:pretend]? → true
    InstallGenerator-->>User: say_status(:pretend, "Skipping...")
    Note over FileSystem: bundle add / bundle install / rails shakapacker:install — NOT run

    InstallGenerator->>InstallGenerator: add_bin_scripts()
    InstallGenerator->>InstallGenerator: directory(...) — Thor pretend-aware, logs only
    InstallGenerator->>InstallGenerator: Dir.chdir / Dir.glob → collect filenames
    InstallGenerator->>InstallGenerator: options[:pretend]? → skip File.chmod
    Note over FileSystem: No files created, no chmod executed

_{Last reviewed commit: b0c71a5}

Review: Fix install generator --pretend side effects and crash

The fix is well-scoped and correctly addresses the two reported issues from #2490. Guarding raw Ruby file operations with options[:pretend] is the right pattern when Thor actions cannot be used.

What is done well

• The ordering in ensure_shakapacker_installed is correct: returning early if Shakapacker is already configured before checking options[:pretend], so pretend mode only intercepts the install path.
• File.chmod is a raw Ruby call that completely bypasses Thor's pretend mechanism, so the unless options[:pretend] guard is necessary and correct.
• Tests are unit-level and targeted, directly exercising the private methods rather than running the full generator.

Potential issues / follow-ups

Issue 1 - Other raw file operations are still not pretend-safe (pre-existing, worth tracking)

The same class has additional raw file/dir operations not guarded by options[:pretend]:

• Line 347: File.write in finalize_shakapacker_setup. Safe for this PR because it is only reachable via ensure_shakapacker_installed, which now returns early in pretend mode. Fragile if the call chain changes later.
• Line 426: FileUtils.mkdir_p for app/javascript/types
• Line 446: File.write for app/javascript/types/css-modules.d.ts
• Line 477: File.write for tsconfig.json

These are pre-existing issues but would still cause real filesystem side effects under --pretend. Worth a follow-up issue.

Issue 2 - add_bin_scripts test does not stub use_rsc?

The test does not stub use_rsc? or gsub_file. Because the generator is initialized without --rsc, use_rsc? returns false and the gsub_file branch is skipped today. Explicitly stubbing use_rsc? to return false would make the test intent clearer and prevent future breakage if default option handling changes.

Issue 3 - Dir.glob is globally mocked in the chmod test

allow(Dir).to receive(:glob).with('*') applies globally for the test duration. Likely fine in practice but broader than needed.

Overall this is a correct, minimal fix. The issues above are minor or pre-existing and should not block merging.

Review: Fix install generator --pretend side effects and crash

This is a well-scoped, correct fix. The crashes and side effects in --pretend mode are real bugs and the guards are placed appropriately.

Overall assessment

The core changes are sound:

• ensure_shakapacker_installed: pretend guard correctly placed after shakapacker_configured? check and before destructive commands.
• add_bin_scripts: File.chmod guard is correct — Thor's directory/gsub_file handle pretend natively, but File.chmod does not.
• create_css_module_types / create_typescript_config: FileUtils/File calls are correctly guarded.

Minor inconsistencies

1. Inconsistent pretend feedback across methods. ensure_shakapacker_installed emits a yellow say_status message in pretend mode. But create_typescript_config returns silently when tsconfig.json does not exist, giving the user no indication of what would have been created. The pattern from ensure_shakapacker_installed is better UX and worth applying consistently.

2. create_css_module_types prints the activity message before the pretend guard. The puts Rainbow line fires before return if options[:pretend], so the user sees a start message but no completion and no pretend annotation. This can look like the method crashed silently. The guard (or a pretend-aware message) should come first.

3. add_bin_scripts test accesses the real template directory. The spec stubs directory and use_rsc? but lets Dir.chdir(template_bin_path) / Dir.glob hit the filesystem. This works because the templates are present in the repo, but it couples the unit test to the layout. Not a blocker.

None of these are regressions — the actual bugs (crash and side effects) are fixed correctly.

Review Summary

The core fix is well-scoped and correct — adding unless options[:pretend] guards to File.chmod calls in both generators eliminates the Errno::ENOENT crash, and the early return in ensure_shakapacker_installed prevents real bundle/install side-effects. The tests cover the new code paths.

A few issues worth addressing:

Bug: install_typescript_dependencies still runs in pretend mode

When using --typescript --pretend, install_typescript_dependencies (install_generator.rb:416–420) still calls add_typescript_dependencies, which delegates to add_packages / add_npm_dependencies and executes real npm/yarn/pnpm commands. The other two TypeScript helpers (create_css_module_types, create_typescript_config) were guarded, but this one was missed.

# invoke_generators (line ~118)
if options.typescript?
  install_typescript_dependencies   # ← no pretend guard, runs npm install
  create_css_module_types           # ← guarded ✓
  create_typescript_config          # ← guarded ✓
end

Recommend adding the same guard to install_typescript_dependencies, or centralising the check at the call site in invoke_generators.

Minor: create_typescript_config checks file existence before the pretend check (install_generator.rb:456–464)

def create_typescript_config
  if File.exist?("tsconfig.json")   # runs in pretend mode too
    puts Rainbow("⚠️  tsconfig.json already exists, skipping creation").yellow
    return
  end

  if options[:pretend]
    say_status :pretend, "Would create tsconfig.json ...", :yellow
    return
  end

In pretend mode with no existing tsconfig.json this is fine. But if tsconfig.json exists and you're in pretend mode, the user sees the "already exists" message rather than any pretend-mode indicator. More importantly, the live File.exist? call happens on every pretend run. Swapping the order (pretend check first, then existence check) would be cleaner and consistent with create_css_module_types.

Nit: test for create_css_module_types stubs methods that the code under test never reaches

it "does not create css module type files in pretend mode" do
  allow(FileUtils).to receive(:mkdir_p)   # ← unreachable in pretend mode
  allow(File).to receive(:write)          # ← unreachable in pretend mode
  ...
end

The method returns before FileUtils.mkdir_p or File.write are called, so the allow stubs are unnecessary. Removing them (and keeping only the negative not_to have_received assertions, which can be dropped since the call path is unreachable) would make the test intention clearer. Minor — not a correctness issue.

Overall this is a good, minimal fix for a real user-facing crash. The install_typescript_dependencies gap is the only functional concern worth addressing before merge.

Review Summary

The fix is well-scoped and correctly addresses the reported --pretend side effects. A few notes:

What's good

• All three raw File.chmod calls that bypass Thor's pretend mode are now guarded correctly (base_generator.rb and install_generator.rb).
• Skipping ensure_shakapacker_installed in pretend mode prevents the real bundle add / bundle install / rails shakapacker:install execution and the resulting .shakapacker_just_installed marker file from ever being written.
• The new guards on create_css_module_types and create_typescript_config close two more pretend-mode gaps for TypeScript setups.
• Test coverage is solid — unit tests for each code path, with correct use of expect(...).not_to receive / not_to have_received assertions.

Minor issues

create_typescript_config guard ordering (install_generator.rb)
The File.exist?("tsconfig.json") check (a read-only operation, so safe in pretend mode) runs before the pretend guard. As a result, when running --pretend with an existing tsconfig, the user sees the puts "⚠️ tsconfig.json already exists…" message instead of a say_status :pretend one. Semantics are correct, but the output style is inconsistent with the rest of the pretend output. Low priority.

Pre-existing gap: handle_custom_webpack_config (base_generator.rb, line 215)
FileUtils.cp(webpack_config_path, backup_path) is a raw file-system call with no pretend guard. This code path is only reached when the user explicitly answers "yes" to the interactive prompt, but in pretend mode that still creates a real backup file. This was not introduced by this PR, but it's now the most visible remaining gap. Worth a follow-up issue/PR.

Verdict

The core bug fix is correct and complete for the reported crash and side effects. The pre-existing webpack-config gap is out of scope here. Approving with the minor notes above for follow-up.

Overall: Good, well-scoped fix for the reported --pretend crash. The core changes are correct and test coverage is reasonable.

What works well: Guards on File.chmod in both install_generator.rb and base_generator.rb directly fix the Errno::ENOENT crash. The ensure_shakapacker_installed pretend guard prevents bundle add/install and rails shakapacker:install side effects. Guards on create_css_module_types and create_typescript_config prevent raw File.write/FileUtils.mkdir_p calls in pretend mode.

One gap: install_typescript_dependencies is not guarded. In invoke_generators (~line 132), when --pretend --typescript is passed, install_typescript_dependencies executes before create_css_module_types and create_typescript_config. The latter two are now guarded but install_typescript_dependencies is not — it calls add_typescript_dependencies -> add_packages -> add_npm_dependencies, which runs real npm/yarn commands. This is the same class of side effect as the Shakapacker installation that is correctly skipped. See inline comment.

Pre-existing issue (out of scope): handle_custom_webpack_config in base_generator.rb line 223 has FileUtils.cp(webpack_config_path, backup_path) without a pretend guard — worth a follow-up.