-
Notifications
You must be signed in to change notification settings - Fork 0
Description
Generator Conflict Resolution: Lessons Learned
This issue documents the investigation and lessons learned from react_on_rails PR #1848, which attempted to automatically resolve file conflicts during the React on Rails generator installation.
Original Problem
When running rails generate react_on_rails:install on a Rails application with existing default files, the generator would pause with interactive prompts asking whether to overwrite:
bin/dev(from Rails 7+)config/shakapacker.yml(from Shakapacker installation)
This caused:
- Interrupted automated installations - CI/CD pipelines would hang waiting for user input
- Required manual intervention - Users had to manually respond during installation
Attempted Solution
Auto-detect and remove these files before the installer runs, but only if they matched the exact default content from their respective gems:
- Check if
bin/devmatches Rails default → auto-remove - Check if
config/shakapacker.ymlmatches Shakapacker default → auto-remove - Preserve any customized versions
- Proceed without interruption
Initial Implementation
def remove_default_gem_files
remove_default_bin_dev
remove_default_shakapacker_yml
end
def remove_default_bin_dev
bin_dev_path = File.join(destination_root, "bin/dev")
return unless File.exist?(bin_dev_path)
# Hardcoded Rails default (THIS WAS WRONG!)
default_bin_dev = <<~RUBY.strip
#!/usr/bin/env ruby
exec "./bin/rails", "server", *ARGV
RUBY
current_content = File.read(bin_dev_path).strip
return unless current_content == default_bin_dev
File.delete(bin_dev_path)
end
def remove_default_shakapacker_yml
config_path = File.join(destination_root, "config/shakapacker.yml")
return unless File.exist?(config_path)
return unless shakapacker_yml_matches_default?(config_path)
File.delete(config_path)
end
def shakapacker_yml_matches_default?(config_path)
shakapacker_gem_path = Gem.loaded_specs["shakapacker"]&.full_gem_path
return false unless shakapacker_gem_path
default_config_path = File.join(shakapacker_gem_path, "lib/install/config/shakapacker.yml")
return false unless File.exist?(default_config_path)
default_content = File.read(default_config_path)
current_content = File.read(config_path)
current_content == default_content
endWhy It Failed
1. Timing/Race Conditions
The generator orchestration was complex:
run_generatorscallsremove_default_gem_filesearly- Then calls
invoke_generatorswhich triggersshakapacker:install shakapacker:installcreatesconfig/shakapacker.yml- React on Rails generator tries to copy its own version
- If deleted at the wrong time → "Shakapacker configuration file not found" errors
2. Hardcoded Defaults Were Incorrect
The bin/dev comparison used a hardcoded Ruby snippet:
#!/usr/bin/env ruby
exec "./bin/rails", "server", *ARGVBut Rails actually generates a shell script from a template. The comparison would never match, making the feature completely ineffective.
3. Detection Complexity
Accurately distinguishing between:
- Pre-existing files (should maybe delete)
- Just-created-by-installer files (should never delete)
- Customized files (must preserve)
This proved extremely difficult in the generator execution context.
4. Path Complexity Issues
Even "simple" fixes like changing:
gemfile = ENV["BUNDLE_GEMFILE"] || "Gemfile"to:
gemfile = ENV["BUNDLE_GEMFILE"] || File.join(destination_root, "Gemfile")Added unnecessary complexity. Rails generators run with the Rails root as working directory, making the simpler string path more reliable.
Code Review Findings
Multiple reviewers (CodeRabbit and manual review) identified:
- Hardcoded bin/dev content doesn't match Rails template - Fatal flaw
- Race conditions during file deletion - Timing issues
- Unnecessary path complexity - Over-engineering
- Better alternatives exist - See below
Better Alternatives
Instead of auto-removing files, consider:
Option 1: Use --force Flag
# In automated scenarios
rails generate react_on_rails:install --forceThis skips all conflict prompts (though may overwrite customizations).
Option 2: Pre-Installation Cleanup Script
Provide a documented cleanup script:
# bin/prepare-react-on-rails
#!/bin/bash
# Run before installing React on Rails
rm -f bin/dev config/shakapacker.yml
rails generate react_on_rails:installOption 3: Better Documentation
Document the conflict and provide clear instructions:
## Installation on Existing Rails Apps
If you see conflict prompts for `bin/dev` or `config/shakapacker.yml`:
1. If these are default, unmodified files, choose `Y` to overwrite
2. If you've customized them, choose `n` and manually merge later
3. For automated installs, use `--force` flagOption 4: Check for Conflicts Before Installation
Warn users upfront:
def installation_prerequisites_met?
check_for_potential_conflicts
# ... other checks
end
def check_for_potential_conflicts
if File.exist?("config/shakapacker.yml")
GeneratorMessages.add_warning(<<~MSG)
⚠️ config/shakapacker.yml already exists.
You may see a conflict prompt. To avoid:
• Backup your file if customized
• Run: rm config/shakapacker.yml
• Or use: --force flag (overwrites without asking)
MSG
end
endKey Lessons
- Simpler is Better: The "clever" automated solution created more problems than it solved
- Don't Hardcode Templates: Always read actual gem templates dynamically
- Generator Timing is Complex: File operations in generators have subtle ordering dependencies
- Trust Rails Conventions: Simple paths like
"Gemfile"are often better thanFile.join(destination_root, "Gemfile") - User Control > Automation: Sometimes explicit user decisions (even with prompts) are better than automatic "magic"
- Alternative Solutions: Interactive prompts aren't the only way - flags, documentation, and scripts can solve the problem
Final Outcome
The PR was closed with no changes after:
- Implementing the auto-remove feature
- Multiple refinement iterations
- Persistent CI test failures
- Reverting the entire feature
- Reverting even the "simple" Gemfile path fix
Zero net changes was the right outcome - the original code was simpler and more maintainable.
Related Links
- Original PR: fix: Auto-remove default gem files to prevent installer conflicts react_on_rails#1848
- Initial commit: shakacode/react_on_rails@34a66d81
- Final revert: shakacode/react_on_rails@4fd39fc0
Recommendation
For react_on_rails-demos and documentation:
- Document the potential for conflict prompts during installation
- Provide clear instructions for handling them
- Consider adding a
bin/prepare-react-on-railsscript for automated setups - Do not attempt to auto-remove files during generator execution
This issue serves as a reference for future discussions about generator improvements and conflict resolution strategies.