Add --pro and --rsc flags to install generator #2284
Conversation
WalkthroughThis PR implements the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant InstallGenerator
participant ProGenerator
participant RscGenerator
participant FileSystem
participant NPM
User->>InstallGenerator: rails g react_on_rails:install --pro --rsc
InstallGenerator->>InstallGenerator: validate prerequisites
InstallGenerator->>ProGenerator: invoke with pro: true
ProGenerator->>FileSystem: create react_on_rails_pro.rb
ProGenerator->>FileSystem: create node-renderer.js
ProGenerator->>FileSystem: modify Procfile.dev
ProGenerator->>FileSystem: update serverWebpackConfig.js
ProGenerator->>NPM: install react-on-rails-pro packages
InstallGenerator->>RscGenerator: invoke with rsc: true
RscGenerator->>FileSystem: create rscWebpackConfig.js
RscGenerator->>FileSystem: create HelloServer components
RscGenerator->>FileSystem: create hello_server_controller
RscGenerator->>FileSystem: add RSC routes
RscGenerator->>FileSystem: update Procfile.dev
RscGenerator->>FileSystem: update serverWebpackConfig.js with RSC plugin
RscGenerator->>NPM: install RSC packages
InstallGenerator->>NPM: install React & JS dependencies
User->>FileSystem: app ready with Pro & RSC
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Code Review: Add --pro and --rsc flags to install generatorGreat work on implementing the ✅ Strengths1. Excellent Test Coverage
2. Strong Error Handling & User Experience
3. Clean Architecture
4. Smart Package Management
🔍 Issues & SuggestionsCritical: Security - Hardcoded Password in Template
config.renderer_password = ENV.fetch("RENDERER_PASSWORD", "devPassword")Issue: Using a hardcoded fallback password ( Recommendation: # In production, require the password. In dev, allow fallback.
config.renderer_password = if Rails.env.production?
ENV.fetch("RENDERER_PASSWORD")
else
ENV.fetch("RENDERER_PASSWORD", "devPassword")
endOr generate a random password during installation: # In the generator
password = SecureRandom.hex(32)
# Write to .env file or Rails credentialsCode Quality Issues1. Inconsistent React Version Warning Logic
if major != 19 || minor != 0
# Warns for 19.1.x and 18.x
GeneratorMessages.add_warning(...)
elsif patch < 3
# Warns for 19.0.0, 19.0.1, 19.0.2
GeneratorMessages.add_warning(...)
endIssue: If someone has React 19.1.0, they get the "RSC requires 19.0.x" warning, but not the CVE warning (even though 19.1.0 might also have vulnerabilities). The logic should be more explicit. Recommendation: if major != 19 || minor != 0
# Version incompatibility (wrong major.minor)
GeneratorMessages.add_warning(...)
elsif major == 19 && minor == 0 && patch < 3
# CVE warning for vulnerable 19.0.x versions
GeneratorMessages.add_warning(...)
end2. Potential Bug: Version Detection Doesn't Handle Pre-release Versions
match = react_version.match(/(\d+)\.(\d+)\.(\d+)(?:[-.]([0-9A-Za-z.-]+))?/)
return nil unless match
"#{match[1]}.#{match[2]}.#{match[3]}" # Drops pre-release suffixIssue: The comment says it handles beta versions like Recommendation: Either:
3. Missing ENV Variable Documentation
Recommendation: Add a comment block at the top of the file documenting all available ENV variables and their defaults, or reference documentation. 4. Potential Race Condition in Procfile Check
if File.read(procfile_path).include?("node-renderer:")Issue: This checks for the exact string Recommendation: Use a more robust regex: if File.read(procfile_path).match?(/^\s*node-renderer:/m)Best Practices & Maintainability1. Magic Numbers
Recommendation: Consider extracting to a constant or documenting why this specific port was chosen. 2. TODO Comment Should Be Tracked
# TODO: Update URL to licenses.shakacode.com when the self-service licensing app is deployedRecommendation: Create a GitHub issue to track this TODO so it doesn't get lost. 3. Test Comment Needs Action
# TODO: When --rsc tests are added, evaluate if this negative test is redundant.Recommendation: Since you mentioned Phase 3 is for RSC generator, add a checkbox in your PR description to revisit this test once RSC tests are implemented. 🎯 Performance ConsiderationsDependency Installation Performance
add_react_on_rails_package unless using_pro
add_react_dependencies
add_css_dependencies
add_rspack_dependencies if using_rspack?
add_swc_dependencies if using_swc?
add_pro_dependencies if using_pro
add_dev_dependenciesCurrent behavior: Each Observation: According to your module documentation (line 30), this is intentional and safe because package_json gem's install is idempotent. This is fine, but consider whether batching all package additions before any writes would be cleaner. 🔐 Security Considerations1. Environment Variable Injection via Procfile
node_renderer_line = <<~PROCFILE
# React on Rails Pro - Node Renderer for SSR
node-renderer: RENDERER_LOG_LEVEL=info RENDERER_PORT=3800 node client/node-renderer.js
PROCFILEObservation: This hardcodes ENV variables in Procfile.dev. While this is standard practice for Procfile, be aware that anyone with access to the repo can see these values. Recommendation: Add a comment in the generated Procfile.dev pointing users to configure 2. YAML Parsing Security
📝 Documentation & CommentsExcellent Documentation Examples:
Missing Documentation:
🧪 Testing RecommendationsTests Present ✅:
Suggested Additional Tests:
📋 Pre-Merge ChecklistBased on CLAUDE.md requirements:
🎓 SummaryOverall Assessment: This is high-quality work with excellent test coverage and user experience considerations. The main concerns are:
Recommendation: Address the security issue before merging. Other suggestions are nice-to-haves that can be addressed now or in follow-up PRs. Great job on this feature! 🎉 |
Code Review: Add --pro and --rsc flags to install generatorOverall Assessment: ✅ Excellent implementation with strong attention to detail, comprehensive testing, and good developer experience. The code is production-ready with only minor suggestions for improvement. 🎯 Strengths
🔍 Code Quality Issues1. Potential Regex Escaping Issue (generator_helper.rb:108)File.foreach(lockfile).any? { |line| line.match?(/^\s{4}#{Regexp.escape(gem_name)}\s\(/) }✅ Good: Using 2. Error Recovery in detect_react_version (generator_helper.rb:145-164)def detect_react_version
# ...
rescue StandardError
nil
end✅ Good: Graceful degradation on errors 3. Hardcoded Magic Numbers
💡 Suggestion: Consider making these configurable constants or documenting why these specific values were chosen. 4. React Version Validation Logic (install_generator.rb:177-206)if major != 19 || minor != 0
# warning about 19.0.x requirement
elsif patch < 3
# warning about CVE
end✅ Good: Specific version checking with CVE references 🚨 Potential Bugs1. Race Condition in remove_base_package_if_presentdef remove_base_package_if_present
pj = package_json
return unless pj
dependencies = pj.fetch("dependencies", {})
return unless dependencies.key?("react-on-rails")
puts "Removing base 'react-on-rails' package..."
pj.manager.remove(["react-on-rails"]) # ⚠️ Modifies package.json
💡 Recommendation:
🔒 Security Considerations1. Default Password in Templates (node-renderer.js:12, react_on_rails_pro.rb.tt:9)password: env.RENDERER_PASSWORD || 'devPassword',✅ Mitigations:
💡 Suggestion: Add a warning in the generated initializer comment: # IMPORTANT: Change RENDERER_PASSWORD in production!
# See value in client/node-renderer.js
config.renderer_password = ENV.fetch("RENDERER_PASSWORD", "devPassword")2. YAML Safe Loading (generator_helper.rb:170-175)YAML.safe_load_file(path, permitted_classes: [Symbol], aliases: true)✅ Excellent: Using safe_load_file instead of load_file prevents arbitrary code execution ⚡ Performance Considerations1. File I/O in Loops (generator_helper.rb:108)File.foreach(lockfile).any? { |line| ... }✅ Good: Uses 2. Memoization (generator_helper.rb:121)def pro_gem_installed?
return @pro_gem_installed if defined?(@pro_gem_installed)✅ Excellent: Proper memoization prevents repeated gem lookups 📝 Documentation & Usability1. Generator Output Formatting (install_generator.rb:512-516)puts Rainbow("\n#{\'=\' * 80}").cyan
puts Rainbow("🚀 REACT ON RAILS PRO SETUP").cyan.bold
puts Rainbow("=" * 80).cyan✅ Excellent: Clear visual feedback during setup 2. Missing Documentation for --pro FlagThe PR description mentions "Phase 5: Documentation 🔜" - make sure to add:
🧪 Testing Recommendations✅ Well-covered areas:
🎨 Code Style & Conventions✅ Follows project conventions:
💡 Minor style suggestions:
🚀 RecommendationsMust Fix Before Merge:None - code is production-ready! Should Consider:
Nice to Have:
✅ Final VerdictAPPROVED with minor suggestions. This is high-quality code that:
The suggestions above are mostly nitpicks and enhancements rather than blocking issues. Great work! 🎉 CI Status: ✅ All checks passing cc: @ihabadham |
Code Review: Add --pro and --rsc flags to install generatorGreat work on implementing the Pro generator functionality! This is a well-structured PR with good test coverage. Here's my detailed feedback: ✅ Strengths
🐛 Potential Issues1. Missing Template Files in PRThe PR adds references to template files but I couldn't verify their existence:
Action: Verify these template files are included in the PR and committed to git. 2. Missing
|
Code Review: Add --pro and --rsc flags to install generatorOverviewThis is a well-structured PR that adds ✅ Strengths1. Excellent Architecture & Separation of Concerns
2. Comprehensive Test Coverage
3. Robust Error Handling
4. Smart Upgrade Pathdef remove_base_package_if_present
# Automatically removes react-on-rails when installing Pro
# Prevents validation errors from having both packagesThis is a thoughtful UX detail that prevents common migration issues. 5. Security Awareness
🔍 Areas for Improvement1. Hardcoded Security Warnings May Become Stale# react_on_rails/lib/generators/react_on_rails/install_generator.rb:199-205
elsif patch < 3
GeneratorMessages.add_warning(<<~MSG.strip)
⚠️ React #{react_version} has known security vulnerabilities.
Please upgrade to at least React 19.0.3:
See: CVE-2025-55182, CVE-2025-67779
Issue: Hardcoded CVE numbers and version checks will become outdated as new React versions and patches are released. Recommendation: Consider:
2. React Version Detection Regex Duplication# react_on_rails/lib/generators/react_on_rails/generator_helper.rb:157
match = react_version.match(/(\d+)\.(\d+)\.(\d+)(?:[-.]([0-9A-Za-z.-]+))?/)The comment mentions using "the same regex pattern as VersionChecker" but the regex is duplicated. Recommendation: Extract to a shared constant in module ReactOnRails
module VersionChecker
VERSION_REGEX = /(\d+)\.(\d+)\.(\d+)(?:[-.]([0-9A-Za-z.-]+))?/.freeze
end
end3. Package Manager Command in Error Messages# react_on_rails/lib/generators/react_on_rails/js_dependency_manager.rb:299-303
You can install them manually by running:
npm install #{PRO_DEPENDENCIES.join(' ')}Issue: Error messages hardcode Recommendation: Use the detected package manager: manager_name = package_json&.manager&.class&.name&.split('::')&.last&.downcase || 'npm'
"#{manager_name} install #{PRO_DEPENDENCIES.join(' ')}"4. Potential Race Condition in
|
Code Review: Add --pro and --rsc flags to install generatorThis is a comprehensive review of PR #2284. Overall, this is excellent work with solid architecture, good test coverage, and attention to detail. Below are my findings organized by category. ✅ Strengths1. Architecture & Design
2. Code Quality
3. Test Coverage
🔍 Issues FoundCRITICAL: Missing Template FilesThe PR references template files that don't exist in the repository: # install_generator.rb:319
pro_template_path = "templates/pro/base/config/initializers/react_on_rails_pro.rb.tt"
template(pro_template_path, initializer_path)
# install_generator.rb:337
template_path = "templates/pro/base/client/node-renderer.js"
copy_file(template_path, node_renderer_path)Impact: The generator will fail at runtime when these template methods are called. Solution: The template files need to be added to the repository. Based on the PR description and diff, they should contain:
The
|
PR Review: Add --pro and --rsc flags to install generatorSummaryThis PR adds excellent support for automated Pro setup via ✅ Strengths1. Excellent Code Organization
2. Robust Testing
3. Strong Idempotency
4. User-Friendly Error Handling
5. Smart Dependency Management
🔍 Issues & Suggestions🔴 Critical Issues1. Gemfile.lock Parsing Regex May Be Too StrictFile: File.foreach(lockfile).any? { |line| line.match?(/^\s{4}#{Regexp.escape(gem_name)}\s\(/) }Issue: This assumes exactly 4 spaces of indentation. While this matches standard Bundler output, it's fragile:
Recommendation: File.foreach(lockfile).any? { |line| line.match?(/^\s+#{Regexp.escape(gem_name)}\s+\(/) }Use 2. React Version Detection Doesn't Handle All Semver FormatsFile: Issue: The regex doesn't handle:
Recommendation: # Strip leading semver operators first
cleaned_version = react_version.gsub(/^[\^~>=<\s]+/, '')
# Then extract first version number found
match = cleaned_version.match(/(\d+)\.(\d+)\.(\d+)(?:[-.]([0-9A-Za-z.-]+))?/)
|
Code Review: PR #2284 - Add --pro and --rsc flags to install generatorOverviewThis PR adds ✅ Strengths1. Excellent Error Handling & User Guidance
2. Strong Test Coverage
3. Good Code Organization
4. Smart Package Management
|
Code Review: Add --pro and --rsc flags to install generatorThis is an excellent implementation with high code quality, comprehensive test coverage, and thoughtful user experience design. Below is my detailed review: ✅ Strengths1. Code Quality & Architecture
2. User Experience
3. Test Coverage
4. Security & Best Practices
🔍 Issues & RecommendationsCritical: Missing Template FilesThe PR references template files that don't appear to exist in the repository:
Action Required: These files need to be included in the commit. I verified they don't exist using Minor: Inconsistent Method Naming
# Current
def using_rspack?
respond_to?(:options) && options&.rspack?
end
# Suggested (for consistency with use_pro?, use_rsc?)
def use_rspack?
respond_to?(:options) && options&.rspack?
endEnhancement: Extract Magic Numbers
REACT_RSC_MAJOR = 19
REACT_RSC_MINOR = 0
REACT_RSC_MIN_SECURE_PATCH = 3
if major != REACT_RSC_MAJOR || minor != REACT_RSC_MINOR
# warning...
elsif patch < REACT_RSC_MIN_SECURE_PATCH
# security warning...
endCode Style: Ternary Consistency
# Current
!(missing_node? || missing_package_manager? || missing_pro_gem? ||
ReactOnRails::GitUtils.uncommitted_changes?(GeneratorMessages))
# Suggested
return false if missing_node?
return false if missing_package_manager?
return false if missing_pro_gem?
!ReactOnRails::GitUtils.uncommitted_changes?(GeneratorMessages)Documentation GapThe Pro initializer template uses 🔒 Security Review✅ No security concerns found
🎯 Performance Considerations✅ Well-optimized
📋 Testing RecommendationsBefore Merge:
CI Verification:
📚 Adherence to Project Guidelines✅ Excellent compliance with CLAUDE.md:
🎖️ Overall AssessmentRating: 9/10 (would be 10/10 with template files) This is production-ready code with one critical blocker (missing templates). The implementation is:
Recommendation: ✅ Approve after adding template files 🚀 Next Steps
Great work on this feature! The code quality and test coverage are exemplary. Just need those template files and this is ready to ship. 🚀 |
Code Review: Add --pro and --rsc flags to install generatorExecutive SummarySolid implementation with good test coverage! I've identified several bugs and security considerations that should be addressed before merge. Overall architecture is sound and follows React on Rails conventions well. Critical Bugs to Fix1. Version Detection Regex Does Not Handle PrefixesLocation: generator_helper.rb:158 Problem: The regex fails to extract versions from common patterns like "^19.0.3" or "~19.0.3" The current code only checks for "/" and "workspace:" but does not strip version prefixes before matching. Fix: Strip common npm version prefixes (^, ~, >=, etc.) before extracting version numbers. 2. Package Removal Race ConditionLocation: js_dependency_manager.rb:298-318 Problem: remove_base_package_if_present reads cached package.json which may not reflect recent changes. Fix: Force reload before checking by setting @package_json = nil first. 3. Silent Error Swallowing in Version DetectionLocation: generator_helper.rb:165 Fix: Add debug logging when errors occur so developers can diagnose issues. 4. Template Overwrite RiskLocation: install_generator.rb:535-560 Fix: Use explicit force: false parameter when calling template() to prevent accidental overwrites. Security ConcernsDefault Password Warning NeededBoth templates use devPassword as default. Should add post-install warning message advising users to change this for production deployments. Recommend adding a GeneratorMessages.add_warning with instructions to:
Test Coverage GapsMissing Edge Cases
Code Quality Improvements1. Extract Magic Numbers to ConstantsLocation: install_generator.rb:169-192 Replace hardcoded 19, 0, 3 with named constants:
2. Reduce Test DuplicationMultiple Pro test contexts repeat identical assertions. Create shared_examples for pro_generator_files. What Is Done Well
Pre-Merge Checklist
Overall AssessmentQuality: Strong (4/5 stars) Great work on this feature! The architecture is solid, tests are comprehensive, and it follows React on Rails conventions well. Fix the critical regex bug and add the security warning before merge. |
Code Review - PR #2284: Add --pro and --rsc flags to install generatorI've reviewed the changes in this PR. Overall, this is a well-structured implementation with good test coverage. Here are my findings: ✅ Strengths
🔍 Issues & Suggestions1. Critical: Missing Template Files in RepositoryThe PR adds new template files that need to be verified:
Verification needed: # Ensure these files exist and are tracked by git
ls -la react_on_rails/lib/generators/react_on_rails/templates/pro/base/client/
ls -la react_on_rails/lib/generators/react_on_rails/templates/pro/base/config/initializers/2. Potential Bug: Package Removal Race ConditionIn def remove_base_package_if_present
pj = package_json
return unless pj
dependencies = pj.fetch("dependencies", {})
return unless dependencies.key?("react-on-rails")
puts "Removing base 'react-on-rails' package..."
pj.manager.remove(["react-on-rails"]) # ⚠️ This might fail if package.json was modified
puts "✅ Removed 'react-on-rails' package"Issue: No verification that removal succeeded before printing success message. Suggested fix: result = pj.manager.remove(["react-on-rails"])
puts "✅ Removed 'react-on-rails' package" if result3. Code Smell: Duplicate Code in Regex PatternIn match = react_version.match(/(\d+)\.(\d+)\.(\d+)(?:[-.]([0-9A-Za-z.-]+))?/)The comment mentions "Uses VERSION_PARTS_REGEX pattern from VersionChecker for consistency" but defines a new regex inline. This creates maintenance burden if the pattern needs to change. Suggested refactor: # Extract to shared constant in VersionChecker
ReactOnRails::VersionChecker::VERSION_PARTS_REGEX = /(\d+)\.(\d+)\.(\d+)(?:[-.]([0-9A-Za-z.-]+))?/
# Then use in both places
match = react_version.match(ReactOnRails::VersionChecker::VERSION_PARTS_REGEX)4. Missing Edge Case: What if package.json doesn't exist?In def detect_react_version
pj = package_json
return nil unless pj
dependencies = pj.fetch("dependencies", {}) # ⚠️ Could raise KeyErrorIssue: dependencies = pj["dependencies"] || {}
# or
dependencies = pj.dig("dependencies") || {}5. Documentation: Incomplete URL CommentIn # TODO: Update URL to licenses.shakacode.com when the self-service licensing app is deployedSuggestion: Add a GitHub issue number to track this TODO, or create one if it doesn't exist: # TODO(#XXXX): Update URL to licenses.shakacode.com when self-service licensing app is deployed6. Performance: Redundant File ReadIn if File.read(procfile_path).include?("node-renderer:") # Reads entire file
puts Rainbow("ℹ️ Node Renderer already in Procfile.dev, skipping").yellow
return
end
# Later appends to same file
append_to_file("Procfile.dev", node_renderer_line)Minor optimization: For large Procfiles, consider using if File.foreach(procfile_path).any? { |line| line.include?("node-renderer:") }7. Test Coverage Gap: RSC Flag TestsThe PR description mentions:
Observation: The RSC flag is defined but not fully implemented. Consider:
8. Security: Validate React Version StringIn major, minor, patch = react_version.split(".").map(&:to_i)
if major \!= 19 || minor \!= 0
# Warning message
endIssue: No validation that Suggested fix: parts = react_version.split(".")
return unless parts.length == 3
major, minor, patch = parts.map(&:to_i)🎯 Recommendations
📋 Checklist for AuthorBefore merging, please ensure:
🎉 Overall AssessmentThis is high-quality work with excellent test coverage and thoughtful error handling. The issues identified are mostly minor edge cases and potential future improvements. The core functionality is solid and well-tested. Recommendation: Approve with minor fixes suggested above. 🤖 Generated with Claude Code |
Code Review: Add --pro and --rsc flags to install generatorGreat work on this feature! This is a well-structured PR that adds significant value. Strengths
Critical Issues1. Security - Hardcoded PasswordFile: templates/pro/base/client/node-renderer.js:31 2. Procfile Detection BugFiles: install_generator.rb:588-589 and :658 3. Package Removal Race ConditionFile: js_dependency_manager.rb:345-362 4. React Version Validation TimingFile: install_generator.rb:185-218 5. Missing Documentation
Pre-Merge Checklist
Overall AssessmentQuality: 4/5 stars Solid, production-ready code with excellent tests. Address the security issue and missing changelogs before merge. |
Code Review - PR #2284: Add --pro and --rsc flags to install generatorThis is a solid implementation that adds important Pro and RSC installation capabilities to the generator. The code is well-tested, well-documented, and follows best practices. Here's my detailed feedback: ✅ Strengths1. Excellent Test Coverage
2. Well-Designed Architecture
3. Strong User Experience
4. Security & Best Practices
🔍 Issues & RecommendationsCritical Issues1. Hardcoded CVE Numbers May Become Outdated (install_generator.rb:215-216)MSG
⚠️ React #{react_version} has known security vulnerabilities.
Please upgrade to at least React 19.0.3:
npm install react@19.0.3 react-dom@19.0.3
See: CVE-2025-55182, CVE-2025-67779
MSGIssue: Hardcoded CVE numbers with future dates (2025) suggest these may be placeholder values. If these are real CVEs, the message is great. If they're placeholders, they could confuse users. Recommendation:
Medium Priority Issues2. React Version Check Logic Could Be More Robust (install_generator.rb:192-194)major, minor, patch = react_version.split(".").map(&:to_i)
if major \!= 19 || minor \!= 0Issue: If Current mitigation: The Recommendation: Add a defensive check or comment explaining the guarantee: # detect_react_version guarantees major.minor.patch format
major, minor, patch = react_version.split(".").map(&:to_i)3. Missing Validation for Pro Gem Version CompatibilityIssue: The generator checks if Pro gem is installed but doesn't validate version compatibility. If someone has an old Pro gem version, the generator might create incompatible configurations. Recommendation: Consider adding a version check similar to the npm package version validation in 4. Node Renderer Worker Count Logic (templates/pro/base/client/node-renderer.js:36-38)if (env.CI) {
config.workersCount = 2;
}Issue: This overrides the user's Recommendation: if (env.CI && \!env.NODE_RENDERER_CONCURRENCY) {
config.workersCount = 2;
}Minor Issues / Suggestions5. Inconsistent Error Message FormattingSome error messages use emoji (🚫, Example: install_generator.rb:162-177 uses 🚫, but some earlier code may not. Recommendation: Audit all error/warning messages for consistent emoji usage. 6. TODO Comment Should Reference Issue (install_generator.rb:178)# TODO: Update URL to licenses.shakacode.com when the self-service licensing app is deployedRecommendation: Link to a GitHub issue so this TODO is trackable: # TODO(#XXXX): Update URL to licenses.shakacode.com when...7. Procfile.dev Handling Could Be More Robust (install_generator.rb:577-584)Current behavior: Skips with warning if Procfile.dev doesn't exist. Consideration: Should the generator offer to create Procfile.dev? Many Rails apps may not have one yet. Recommendation: Document this requirement in the generator output or offer to create a basic Procfile.dev. 8. SWC Detection Logic (generator_helper.rb:212-236)The Recommendation: Add a comment summarizing the decision flow: # Decision flow:
# 1. If shakapacker.yml exists and specifies transpiler -> use that
# 2. Otherwise, default to SWC for Shakapacker 9.3.0+
# 3. For fresh installs, assume latest Shakapacker (SWC default)9. Package Removal on Upgrade (js_dependency_manager.rb:345-362)The puts Rainbow("🔄 Upgrading from base to Pro: removing 'react-on-rails' package").cyan🧪 Testing RecommendationsWhat's Well-Tested ✅
Additional Test Cases to Consider
📚 Documentation Suggestions
🎯 Performance ConsiderationsMemory & CPU
Generator Speed
🔒 Security Review✅ Good Security Practices
|
Pull Request Review: Add --pro and --rsc Flags to Install GeneratorOverviewThis PR adds Code Quality Assessment: ⭐⭐⭐⭐⭐ ExcellentStrengths1. Architecture & Design
2. Error Handling & UX
3. Testing
4. Code Style
Issues & RecommendationsCritical Issues 🚨None found. This PR is production-ready. Security Considerations ✅
Performance Considerations ✅
Potential Improvements 💡1. Minor: React Version Detection Could Be More Robust# Current implementation in detect_react_version:
return nil if react_version.include?("/") || react_version.start_with?("workspace:")Suggestion: Add detection for other special protocols: # Covers more edge cases
return nil if react_version.match?(/^(workspace:|file:|link:|git\+|https?:\/\/)/)Impact: Low - current implementation handles 95% of cases. 2. Test Coverage: Add Test for Package RemovalThe context "when upgrading from base to Pro with existing react-on-rails package" do
it "removes base react-on-rails package" do
# Test that react-on-rails is removed from dependencies
end
endImpact: Medium - this is important upgrade path logic. Best Practices Adherence✅ Following Repository Guidelines:
✅ Follows CLAUDE.md Requirements:
Final Verdict: ✅ APPROVED - Ready to MergeThis is excellent work. The implementation is:
Specific Callouts:
Great job! This is a significant feature addition that will make Pro adoption much smoother for users. |
ihabadham
force-pushed
the
ihabadham/feature/pro-rsc-generator
branch
from
506aab7 to
5913009
Compare
ihabadham
force-pushed
the
ihabadham/feature/pro-rsc-generator
branch
from
5913009 to
2fa0366
Compare
PR Review: Add --pro and --rsc flags to install generatorOverviewThis PR adds ✅ Strengths1. Excellent Code Organization
2. Comprehensive Test CoverageThe test suite covers:
3. Strong Error Handling
4. Idempotency by Design
5. User Experience
|
Comprehensive Code Review for PR #2284Thank you for this well-structured PR! Overall, this is solid work with excellent attention to UX and error handling. Critical Issues1. Missing Error Handling in gsub_file (rsc_setup.rb:127) 2. React Version Check Timing 3. Missing Pro Gem Version Validation Potential Bugs
Testing GapsMissing integration tests for:
What This PR Does Really Well✅ Excellent module separation (ProSetup, RscSetup) ArchitectureThe separation into ProSetup and RscSetup modules is exactly right - this is a model for structuring complex generators. Pre-Merge Checklist
ConclusionHigh-quality work with sound architecture. Issues are mostly edge cases and safety improvements. Once integration tests are added and critical issues addressed, this will be a solid addition. Great job! |
ihabadham
force-pushed
the
ihabadham/feature/pro-rsc-generator
branch
from
7cc057b to
56525a4
Compare
This commit implements Phase 1 of the Pro/RSC generator feature: Generator flags: - --pro: Install React on Rails Pro with Node Renderer - --rsc: Install React Server Components support (implies --pro) Prerequisite validation: - Checks if react_on_rails_pro gem is installed when flags are used - Shows friendly error message with installation instructions - Includes free trial messaging to lower friction React version validation (RSC only): - Warns if React version is not 19.0.x (required for RSC) - Warns about security vulnerabilities (CVE-2025-55182, CVE-2025-67779) if React version is below 19.0.3 UX improvements: - Added blank lines between generator messages for readability - Improved license messaging ("Try Pro free!") Code quality: - Extracted gem_in_lockfile? helper to eliminate duplication - Fixed memoization pattern in pro_gem_installed? - Reused existing version parsing patterns from VersionChecker 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Test coverage for: - missing_pro_gem? validation (5 tests) - --pro without gem → blocks with error - --rsc without gem → blocks with error - --pro with gem in loaded_specs → allows - --pro with gem in lockfile → allows - no flag → skips check - detect_react_version parsing (4 tests) - standard version, caret prefix, workspace protocol, nil - warn_about_react_version_for_rsc (5 tests) - React 19.0.3 → no warning - React 19.1.x/18.x → incompatibility warning - React 19.0.0-19.0.2 → CVE security warning - no --rsc → skips check 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Creates files and configuration for React on Rails Pro when --pro flag is used: Files created: - config/initializers/react_on_rails_pro.rb - Pro configuration - client/node-renderer.js - Node Renderer bootstrap Procfile.dev: - Appends node-renderer process (idempotent) NPM packages: - react-on-rails-pro - react-on-rails-pro-node-renderer Implementation: - setup_pro orchestrates Pro file generation - Idempotency checks skip existing files - Pro dependencies added in add_js_dependencies flow (single npm install) - RSC config conditionally included when --rsc flag used Templates based on: - react_on_rails_pro/spec/dummy/ (actual patterns) - react_on_rails_pro/docs/installation.md (config options) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
When users upgrade from the base React on Rails to Pro: - The validation now provides clear upgrade instructions with the REACT_ON_RAILS_SKIP_VALIDATION=true command - The generator automatically removes the base 'react-on-rails' npm package when installing Pro, since Pro includes all base functionality This enables a smooth upgrade path: 1. Add react_on_rails_pro gem to Gemfile 2. Run: REACT_ON_RAILS_SKIP_VALIDATION=true rails g react_on_rails:install --pro 3. Generator removes base package and installs Pro packages 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Tests cover: - Pro initializer creation with NodeRenderer configuration - node-renderer.js bootstrap file generation - Procfile.dev node-renderer process addition - Pro npm dependencies installation - Flag combinations: --pro --redux, --pro --typescript, --pro --rspack - Idempotency: existing Pro files are not overwritten - Unit tests for add_pro_to_procfile behavior This completes the test coverage for the Pro generator implementation (Phase 2 of the --pro/--rsc generator feature). Part of #2277 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
… bundle support Aligns webpack config naming with Pro ecosystem convention. The rename affects the generated config file and all require() references. Adds RSC (React Server Components) three-bundle architecture support: - RSC conditionals in ServerClientOrBoth.js for third bundle - New rscWebpackConfig.js template that extends server config - RSC_BUNDLE_ONLY env var for building RSC bundle separately The RSC webpack config: - Clones server webpack config - Renames entry to rsc-bundle - Adds react-on-rails-rsc/WebpackLoader after JS loader - Adds react-server condition for RSC bundle identification - Aliases react-dom/server to false (not needed for RSC payload) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Adds setup_rsc method that runs when --rsc flag is used: - Creates rscWebpackConfig.js from template - Adds RSC bundle watcher to Procfile.dev Adds react-on-rails-rsc npm package to RSC_DEPENDENCIES in JsDependencyManager, installed alongside other dependencies during generator run. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
- serverWebpackConfig: Add target=node, libraryTarget, RSCWebpackPlugin, extractLoader, node=false for Pro mode - clientWebpackConfig: Add RSCWebpackPlugin for RSC mode - ServerClientOrBoth: Use destructured import for Pro mode - rscWebpackConfig: Use destructured import from serverWebpackConfig - base_generator: Add pro/rsc class options 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
The standalone react_on_rails:pro generator now updates serverWebpackConfig.js to enable Pro settings (libraryTarget: 'commonjs2', target: 'node') which are required for Node Renderer to work with Node.js modules. Previously, these settings were only enabled via ERB conditionals during fresh install with --pro flag, leaving standalone upgrades with broken configs.
Standalone generators (rails g react_on_rails:pro/rsc) modify existing files via gsub_file rather than rendering templates, so they need regex transforms to produce output equivalent to the ERB template conditionals. Pro transforms (pro_setup.rb): - Add extractLoader helper function (required by rscWebpackConfig.js) - Add Babel SSR caller setup (requirement #3 of 4 per Pro docs) - Enable libraryTarget: 'commonjs2' (required for Node Renderer) - Enable target = 'node' with clean comments replacing stale hints - Disable Node.js polyfills (node = false, required for real __dirname) - Change module.exports to object style with extractLoader export - Update ServerClientOrBoth.js to destructured import RSC transforms (rsc_setup.rb): - Add rscWebpackConfig import and config to ServerClientOrBoth.js - Add RSC_BUNDLE_ONLY environment variable handling - Add RSCWebpackPlugin to server and client webpack configs - Add rscBundle parameter to configureServer function Tests (pro_generator_spec.rb, rsc_generator_spec.rb): - Add simulated base/Pro webpack config fixtures so standalone generators have files to transform (previously gsubs silently skipped) - Add content assertions verifying all transform outputs All transforms are idempotent (skip if already applied) and verified against the Pro dummy app, ERB templates, and Pro documentation. Replacement strings use explicit indentation matching the webpack config context (2-space for function bodies, 6-space for nested blocks). Co-Authored-By: Claude Opus 4.6 <[email protected]>
Replace top-level constants (BASE_SERVER_WEBPACK_CONFIG, etc.) with simulate_base_webpack_files and simulate_pro_webpack_files helpers in generator_spec_helper.rb, matching the existing simulate_existing_rails_files pattern. Add clarifying comment to RSC TypeScript context. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Same fix as PR #2288 applied to shakapacker_in_lockfile? — always check the target app's Gemfile.lock directly instead of deriving the path from ENV["BUNDLE_GEMFILE"], which can point to a parent project's Gemfile when running generators via bundle exec. See: #2287 Co-Authored-By: Claude Opus 4.6 <[email protected]>
When using --rsc without --redux, HelloServer replaces HelloWorld as the example component. The post-install message was hardcoded to reference hello_world route and view, which don't exist in RSC-only installs. Now correctly shows hello_server route, view path, and props variable when RSC is enabled without Redux. Co-Authored-By: Claude Opus 4.6 <[email protected]>
The gsub pattern for inserting the RSC_BUNDLE_ONLY else-if block was
consuming the blank line between 'result = serverConfig;' and '} else {'
because \s* greedily matched the newlines. This produced:
result = serverConfig; } else if (process.env.RSC_BUNDLE_ONLY) {
Fixed by matching the leading \n\n outside the capture group and
explicitly restoring it in the replacement string.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
- Add route reference to standalone Pro success message - Fix extractLoader blank line positioning with .chomp - Fix RSC plugin ordering to match template (before LimitChunkCountPlugin) - Fix RSC plugin indentation in serverWebpackConfig (2-space, not 4-space) - Fix clientWebpackConfig regex to not consume blank lines (\s* → ' *') - Add blank line between serverConfig and rscConfig in ServerClientOrBoth - Simplify React version warning to single npm command with ~19.0.3 - Remove stale @note comment about method call ordering These changes ensure standalone gsub transforms produce byte-identical webpack configs compared to the fresh install ERB template path. Co-Authored-By: Claude Opus 4.6 <[email protected]>
ihabadham
force-pushed
the
ihabadham/feature/pro-rsc-generator
branch
from
87087c8 to
dd008fa
Compare
The regex expected \n\n (blank line) before "} else {" but the fixture
content has no blank line there, causing the gsub to silently skip.
Changed to \n to match the actual content structure.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
All other JSX component templates (HelloWorld.client.jsx,
HelloWorldApp.client.jsx) use `const Component = () => {}` style.
HelloServer.jsx was the only one using a function declaration.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
The RSC guards in base_generator.rb unconditionally skipped the hello_world route and controller when --rsc was set. This left --rsc --redux with an orphaned HelloWorldApp view (no route or controller to serve it). Now the guards check for both --rsc and !--redux before skipping. Co-Authored-By: Claude Opus 4.6 <[email protected]>
The bin/dev script hardcoded DEFAULT_ROUTE = "hello_world" which is wrong for --rsc without --redux (should be "hello_server"). Renamed to bin/dev.tt so Thor processes ERB, and added .tt suffix stripping in add_bin_scripts to avoid chmod failure on the wrong filename. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Standalone Pro/RSC generators hardcoded ServerClientOrBoth.js but old installs may still use generateWebpackConfigs.js. Added a shared helper resolve_server_client_or_both_path that detects the legacy name, renames it, and updates env config references. Both pro_setup.rb and rsc_setup.rb now use this helper instead of hardcoded paths. Co-Authored-By: Claude Opus 4.6 <[email protected]>
All gsub transforms silently skip when the regex doesn't match. Added verify_pro_webpack_transforms and verify_rsc_webpack_transforms that re-read the config files after transforms and warn via GeneratorMessages when expected patterns are missing. Co-Authored-By: Claude Opus 4.6 <[email protected]>
The standalone Pro and RSC generator specs passed package_json: true to simulate_existing_rails_files, but that method ignores the param. Added explicit simulate_npm_files calls so package.json exists during test runs. Also added Pro webpack feature assertions to the --pro install context. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Three low-priority fixes: - pro_generator.rb: detect route dynamically instead of hardcoding /hello_world (apps with RSC have /hello_server instead) - js_dependency_manager.rb: use map+all? instead of all? to avoid short-circuiting on first failure (skipping remaining packages) - js_dependency_manager.rb: use PRO_DEPENDENCIES constant in rescue block instead of re-calling pro_packages_with_version (which could also raise) Co-Authored-By: Claude Opus 4.6 <[email protected]>
Document which existing files each standalone generator transforms, so users know what to expect before running the generators. Co-Authored-By: Claude Opus 4.6 <[email protected]>
…lation docs Added details about which webpack configs the Pro and RSC generators modify, the legacy generateWebpackConfigs.js rename, and RSC infrastructure overview so users understand what happens to existing files during upgrade. Co-Authored-By: Claude Opus 4.6 <[email protected]>
The demo repo reference still pointed to webpackConfig.js which was renamed to ServerClientOrBoth.js. Co-Authored-By: Claude Opus 4.6 <[email protected]>
The Pro generator automatically creates client/node-renderer.js, adds the Node Renderer to Procfile.dev, and installs npm packages. Adding a note so users know about this before the manual setup steps. Co-Authored-By: Claude Opus 4.6 <[email protected]>
This reverts commit f4146d5.
…emplate The previous approach renamed bin/dev to bin/dev.tt to make DEFAULT_ROUTE dynamic via ERB. This broke the dummy app (which exec's bin/dev directly) and the dev_spec.rb tests (which read/load the file from source). Instead, keep bin/dev as a plain Ruby file and use gsub_file after copying to change DEFAULT_ROUTE from "hello_world" to "hello_server" when --rsc is used without --redux (the only case where hello_world doesn't exist). Co-Authored-By: Claude Opus 4.6 <[email protected]>
Greptile OverviewGreptile SummaryThis PR adds Key Changes:
Implementation Quality:
Minor Observations:
Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant InstallGenerator
participant ProSetup
participant RscSetup
participant JsDependencyManager
participant FileSystem
User->>InstallGenerator: rails g react_on_rails:install --pro --rsc
InstallGenerator->>InstallGenerator: Validate prerequisites (Pro gem, Node.js)
InstallGenerator->>InstallGenerator: Setup Shakapacker if needed
InstallGenerator->>InstallGenerator: Install TypeScript deps (if --typescript)
InstallGenerator->>InstallGenerator: Invoke base generator
alt Redux flag
InstallGenerator->>InstallGenerator: Generate HelloWorldApp (Redux)
else RSC without Redux
InstallGenerator->>InstallGenerator: Skip HelloWorld generation
else Standard
InstallGenerator->>InstallGenerator: Generate HelloWorld
end
InstallGenerator->>JsDependencyManager: Setup React dependencies
JsDependencyManager->>FileSystem: Install react, react-dom, etc.
InstallGenerator->>ProSetup: setup_pro()
ProSetup->>FileSystem: Create config/initializers/react_on_rails_pro.rb
ProSetup->>FileSystem: Create client/node-renderer.js
ProSetup->>FileSystem: Append node-renderer to Procfile.dev
ProSetup->>FileSystem: Update webpack configs (libraryTarget, extractLoader)
ProSetup->>JsDependencyManager: Install Pro npm packages
JsDependencyManager->>FileSystem: Install react-on-rails-pro, node-renderer
InstallGenerator->>RscSetup: setup_rsc()
RscSetup->>FileSystem: Add RSC config to Pro initializer
RscSetup->>FileSystem: Create config/webpack/rscWebpackConfig.js
RscSetup->>FileSystem: Update ServerClientOrBoth.js (RSC bundle handling)
RscSetup->>FileSystem: Update serverWebpackConfig.js (RSCWebpackPlugin)
RscSetup->>FileSystem: Update clientWebpackConfig.js (RSCWebpackPlugin)
RscSetup->>FileSystem: Create HelloServer component
RscSetup->>FileSystem: Create HelloServerController
RscSetup->>FileSystem: Add RSC routes (rsc_payload_route, hello_server)
RscSetup->>FileSystem: Append rsc-bundle watcher to Procfile.dev
RscSetup->>JsDependencyManager: Install RSC npm packages
JsDependencyManager->>FileSystem: Install react-on-rails-rsc
InstallGenerator->>FileSystem: Add bin scripts (bin/dev, etc.)
InstallGenerator->>User: Display success message with next steps
|
|
@claude review |
|
Claude finished @ihabadham's task in 3m 11s —— View job Code Review for PR #2284Excellent implementation of the ✅ Strengths1. Outstanding Architecture & Code Organization
2. Comprehensive Test Coverage
3. Excellent User Experience
4. Robust Error Handling
🚨 Critical Issues - Must Fix Before Merge1. Missing CHANGELOG Entry ❌According to Required action: Add entries to #### Pro
##### Added
- **Install generator Pro and RSC flags**: New `--pro` and `--rsc` flags for `rails generate react_on_rails:install` enable automated setup of React on Rails Pro with Node Renderer and React Server Components. The `--pro` flag creates Pro initializer, node-renderer.js bootstrap, updates Procfile.dev, and configures webpack for SSR. The `--rsc` flag (which includes Pro) additionally sets up RSC webpack config, HelloServer example component, and RSC routes. Both flags support combinations with existing flags (`--redux`, `--typescript`, `--rspack`). [PR 2284](https://github.com/shakacode/react_on_rails/pull/2284) by [ihabadham](https://github.com/ihabadham).Reference:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In `@react_on_rails_pro/docs/installation.md`:
- Line 56: The anchor link "See [License Configuration](`#license-configuration`)
below" is broken because the actual heading is "## License Configuration
(Production Only)" which yields the anchor
"#license-configuration-production-only"; update the link target to match the
generated anchor (change to "#license-configuration-production-only") or
alternatively rename the heading to "## License Configuration" so the existing
link "#license-configuration" works—locate the link text on Line 56 and the
heading "## License Configuration (Production Only)" to apply the fix.
In `@react_on_rails/lib/generators/react_on_rails/rsc_setup.rb`:
- Around line 248-266: The add_rsc_routes method reads config/routes.rb into
routes_content without checking existence, which can raise Errno::ENOENT; guard
the read by checking File.exist?(routes_file) (or early-return with a log)
before calling File.read so add_rsc_routes behaves like the other methods; keep
the existing behavior of skipping when routes already contain
"rsc_payload_route" and still call route "rsc_payload_route" and route "get
'hello_server', to: 'hello_server#index'" only when the file is present.
- Around line 78-86: The GeneratorMessages.add_warning block in rsc_setup.rb
currently recommends upgrading to [email protected]/[email protected] but that leaves
CVE-2025-67779 unaddressed; update the recommended minimum to [email protected] and
[email protected] in the GeneratorMessages.add_warning message (reference the
react_version interpolation and the warning string) so the printed guidance
addresses both CVE-2025-55182 and CVE-2025-67779.
In
`@react_on_rails/lib/generators/react_on_rails/templates/pro/base/client/node-renderer.js`:
- Around line 14-15: The current assignment for workersCount uses
Number(env.NODE_RENDERER_CONCURRENCY) || 3 which treats 0 as falsy and overrides
a deliberate "0" setting; change the logic in the workersCount initialization so
a provided "0" is preserved (e.g., use a nullish check or explicit undefined
check), converting the env value to Number only when present: if
env.NODE_RENDERER_CONCURRENCY is null/undefined default to 3, otherwise set
workersCount = Number(env.NODE_RENDERER_CONCURRENCY) (and optionally validate
it’s a safe integer). Ensure this update is applied to the workersCount
expression so single-process mode (0) is honored.
In
`@react_on_rails/lib/generators/react_on_rails/templates/pro/base/config/initializers/react_on_rails_pro.rb.tt`:
- Line 10: The current default using ENV.fetch("RENDERER_PASSWORD",
"devPassword") will allow a hardcoded password into production; update the
initializer that sets config.renderer_password so it does not silently default
outside development: retrieve ENV["RENDERER_PASSWORD"] (or ENV.fetch without a
default) and if the value is blank and Rails.env is not development/test raise a
clear error (or abort) explaining RENDERER_PASSWORD must be set; alternatively,
keep the development fallback only when Rails.env.development? and add a comment
above config.renderer_password documenting that the variable is required in
production. Ensure you modify the code path that currently references
config.renderer_password and ENV.fetch("RENDERER_PASSWORD", "devPassword").
In `@react_on_rails/lib/react_on_rails/engine.rb`:
- Around line 72-77: The comment above the self.running_generator? method
overstates Rails' guarantee; update the comment to explain that the check
(defined?(Rails::Generators)) is a heuristic/fallback rather than a strict
guarantee — Rails::Generators can be defined whenever code calls require
"rails/generators" (e.g., test helpers or gems/initializers), so
running_generator? may be true outside of generator commands; keep mention that
this is behind the ENV check and is limited in practice but clarify the
limitation and potential false positives instead of implying Rails only loads
the module during generator execution.
In `@react_on_rails/spec/dummy/Procfile.dev`:
- Around line 4-5: Replace the inconsistent boolean value in the Procfile.dev
entry so it matches the project template: update the server-bundle line that
currently sets SERVER_BUNDLE_ONLY=true to use SERVER_BUNDLE_ONLY=yes; this
change is in the Procfile.dev “server-bundle” entry and will align it with the
template at lib/generators/react_on_rails/templates/base/base/Procfile.dev.
In `@react_on_rails/spec/react_on_rails/generators/install_generator_spec.rb`:
- Around line 1069-1127: Add a before hook in each of the four prerequisite
validation contexts to call GeneratorMessages.clear so tests don't leak
class-level messages between examples: for the contexts that define
let(:install_generator) with flags (pro/rsc) and the one without flags, add
before { GeneratorMessages.clear } above the existing stubs/allow calls so
missing_pro_gem? and subsequent assertions operate on a clean message state;
reference the contexts using the install_generator let and the missing_pro_gem?
method and ensure the before hook is placed consistently as done around line 916
in other specs.
🧹 Nitpick comments (4)
react_on_rails/spec/react_on_rails/engine_spec.rb (1)
129-137: Tautological test — always passes regardless of state.The test computes
expected = defined?(Rails::Generators)and asserts the method returns the same value. Since the method's implementation isdefined?(Rails::Generators), this is a tautology — it passes whetherRails::Generatorsis loaded or not, and never exercises the alternate branch.To meaningfully test both paths, you'd need to test with and without
Rails::Generatorsdefined. Since undefining a Rails constant is fragile, an alternative is to simply assert the expected value in your test environment (likely truthy in RSpec) and add a comment acknowledging the limitation.♻️ Suggested improvement
it "uses defined?(Rails::Generators) for detection" do - result = described_class.running_generator? - expected = defined?(Rails::Generators) - expect(result).to eq(expected) + # In the test environment, Rails::Generators is typically loaded, + # so this verifies the method reflects that state. + # The false branch (Generators not loaded) only occurs in + # non-generator Rails processes (server, console, etc.). + expect(described_class.running_generator?).to be_truthy endreact_on_rails/lib/generators/react_on_rails/templates/base/base/config/webpack/ServerClientOrBoth.js.tt (1)
16-18: Inconsistent ERB tag whitespace control (<% %>vs<% -%>).Lines 16, 21, 38, 47 use
<% if use_rsc? %>(no trailing dash), while lines 4, 8, 9, 11 use<% -%>. The no-dash tags leave extra blank lines in the generated JS output. Not a functional issue, but for cleaner generated files, consider using-%>consistently.Also applies to: 38-53
react_on_rails/lib/generators/react_on_rails/rsc_setup.rb (1)
447-453:check_rsc_scob_configmay trigger side effects viaresolve_server_client_or_both_path.Per the relevant code snippet in
generator_helper.rb(lines 229-247),resolve_server_client_or_both_pathrenamesgenerateWebpackConfigs.js→ServerClientOrBoth.jsand performsgsub_fileon environment files. Calling it from a verification method (verify_rsc_webpack_transforms) is unexpected — it could rename files during what should be a read-only check pass. On the second call in the same run (first fromupdate_server_client_or_both_for_rscat Line 288, then here at Line 448), the rename will already be done so this is benign. But in edge cases (e.g., ifupdate_server_client_or_both_for_rscwas skipped), the side effect would be surprising.Consider caching the resolved path or using a read-only check here.
react_on_rails/lib/generators/react_on_rails/pro_setup.rb (1)
165-220: Stalecontentvariable passed to sub-methods after file mutations.
contentis read once on line 173, then passed toadd_extract_loader_to_server_configandadd_babel_ssr_caller_to_server_config. After the first method callsgsub_fileand mutates the file on disk,contentno longer reflects the actual file. This works today because both sub-methods only usecontentfor skip-checks (checking whether a pattern is already present), andgsub_filere-reads the file internally. However, a future maintainer adding logic that relies oncontentreflecting the current file state could introduce a bug.Consider adding a brief comment on line 173 clarifying this is intentional:
- content = File.read(webpack_config_path) + # Read once for skip-checks only; gsub_file re-reads the file for each transform + content = File.read(webpack_config_path)
| export REACT_ON_RAILS_PRO_LICENSE="your-license-token-here" | ||
| ``` | ||
|
|
||
| See [License Configuration](#license-configuration) below for other options. |
There was a problem hiding this comment.
Broken anchor link: #license-configuration doesn't match the actual heading.
The heading on Line 104 is ## License Configuration (Production Only), which generates the anchor #license-configuration-production-only. The link on Line 56 will not navigate correctly.
📝 Proposed fix
-See [License Configuration](`#license-configuration`) below for other options.
+See [License Configuration](`#license-configuration-production-only`) below for other options.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| See [License Configuration](#license-configuration) below for other options. | |
| See [License Configuration](`#license-configuration-production-only`) below for other options. |
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 56-56: Link fragments should be valid
(MD051, link-fragments)
🤖 Prompt for AI Agents
In `@react_on_rails_pro/docs/installation.md` at line 56, The anchor link "See
[License Configuration](`#license-configuration`) below" is broken because the
actual heading is "## License Configuration (Production Only)" which yields the
anchor "#license-configuration-production-only"; update the link target to match
the generated anchor (change to "#license-configuration-production-only") or
alternatively rename the heading to "## License Configuration" so the existing
link "#license-configuration" works—locate the link text on Line 56 and the
heading "## License Configuration (Production Only)" to apply the fix.
| GeneratorMessages.add_warning(<<~MSG.strip) | ||
| ⚠️ React #{react_version} has known security vulnerabilities. | ||
|
|
||
| Please upgrade to at least React 19.0.3: | ||
| npm install [email protected] [email protected] | ||
|
|
||
| See: CVE-2025-55182, CVE-2025-67779 | ||
| MSG | ||
| end |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
CVE-2025-55182 React security vulnerability
💡 Result:
CVE-2025-55182 (aka “React2Shell”) is a critical, unauthenticated pre-auth remote code execution (RCE) vulnerability in React Server Components (RSC). Attackers can send a specially crafted HTTP request to React Server Function / Server Action endpoints, exploiting unsafe decoding/deserialization in the RSC (“Flight”) request handling to execute arbitrary code on the server. [1][2][3]
Affected components / versions
Vulnerable (per React / NVD):
react-server-dom-webpack19.0.0, 19.1.0, 19.1.1, 19.2.0react-server-dom-parcel19.0.0, 19.1.0, 19.1.1, 19.2.0react-server-dom-turbopack19.0.0, 19.1.0, 19.1.1, 19.2.0 [1][2]
Important note: Even if your app doesn’t define Server Functions directly, it may still be vulnerable if it supports/bundles RSC. [1]
Fixed versions
React states the fix is in: 19.0.1, 19.1.2, 19.2.1 (upgrade to a fixed version immediately). [1]
Next.js impact (downstream)
Next.js confirms this originates upstream in React (CVE-2025-55182) and tracks Next.js impact separately (CVE-2025-66478), affecting App Router deployments on:
- Next.js 15.x
- Next.js 16.x
- Next.js 14.3.0-canary.77+ (canary) [4]
Exploitation status
- Microsoft reports observed exploitation activity beginning in early December 2025. [3]
- CISA added CVE-2025-55182 to its Known Exploited Vulnerabilities (KEV) catalog on December 5, 2025 (indicating active exploitation). [5]
What to do
- Identify any use of RSC /
react-server-dom-*packages in production. [1][2] - Patch immediately: upgrade React/RSC packages to 19.0.1 / 19.1.2 / 19.2.1 (or later). [1]
- If using Next.js App Router, upgrade to a patched Next.js release per Next.js guidance (and consider secret rotation if you were exposed while unpatched, per their advisory updates). [4]
Sources: React advisory [1], NVD [2], Microsoft analysis/defense guidance [3], Next.js advisory [4], CISA KEV alert (via archive due to access restrictions) [5].
🌐 Web query:
CVE-2025-67779 React security vulnerability
💡 Result:
CVE-2025-67779 is a high-severity (CVSS 7.5) Denial-of-Service vulnerability in React Server Components (RSC) where a crafted HTTP request to Server Function endpoints can trigger an infinite loop during deserialization, hanging the server process and preventing it from serving requests. [1][2]
Affected
React’s RSC-related packages/paths, including (as called out by React/GitLab):
react-server-dom-webpack,react-server-dom-parcel,react-server-dom-turbopack[1][3]
GitLab specifically flags affected versions including 19.0.2, 19.1.3, 19.2.2 for this CVE. [3]
Fix / what to upgrade to
React’s security post notes that some earlier patches were incomplete, and that fixes were later backported; upgrade to the safe fixed versions:
- 19.0.4, 19.1.5, or 19.2.4 (for the affected RSC packages) [1]
Who is at risk (practical impact)
You’re generally at risk if your deployment supports React Server Components / Server Functions (often via frameworks/bundlers like Next.js) and exposes a server that can receive the crafted request. [1]
Sources: React security blog post (updated Jan 26, 2026) [1], NVD record [2], GitLab Advisory Database [3].
🏁 Script executed:
#!/bin/bash
# Check NIST NVD for these specific CVEs
echo "=== Checking NVD for CVE-2025-55182 ==="
curl -s "https://services.nvd.nist.gov/rest/json/cves/2.0?cveId=CVE-2025-55182" 2>/dev/null | head -20
echo ""
echo "=== Checking NVD for CVE-2025-67779 ==="
curl -s "https://services.nvd.nist.gov/rest/json/cves/2.0?cveId=CVE-2025-67779" 2>/dev/null | head -20Repository: shakacode/react_on_rails
Length of output: 31521
Update recommended React versions to address incomplete vulnerability fix.
Lines 84-85 reference two legitimate React Server Components vulnerabilities (CVE-2025-55182 and CVE-2025-67779), but the recommended upgrade to [email protected] [email protected] is insufficient. Version 19.0.3 addresses CVE-2025-55182 but remains vulnerable to CVE-2025-67779 (which affects 19.0.2 and requires 19.0.4+). Update the recommendation to [email protected] [email protected] minimum to address both vulnerabilities.
🤖 Prompt for AI Agents
In `@react_on_rails/lib/generators/react_on_rails/rsc_setup.rb` around lines 78 -
86, The GeneratorMessages.add_warning block in rsc_setup.rb currently recommends
upgrading to [email protected]/[email protected] but that leaves CVE-2025-67779
unaddressed; update the recommended minimum to [email protected] and [email protected]
in the GeneratorMessages.add_warning message (reference the react_version
interpolation and the warning string) so the printed guidance addresses both
CVE-2025-55182 and CVE-2025-67779.
| def add_rsc_routes | ||
| routes_file = File.join(destination_root, "config/routes.rb") | ||
| routes_content = File.read(routes_file) | ||
|
|
||
| if routes_content.include?("rsc_payload_route") | ||
| puts Rainbow("ℹ️ RSC routes already exist, skipping").yellow | ||
| return | ||
| end | ||
|
|
||
| puts Rainbow("📝 Adding RSC routes...").yellow | ||
|
|
||
| # Add rsc_payload_route (required for RSC payload requests) | ||
| route "rsc_payload_route" | ||
|
|
||
| # Add HelloServer route (RSC counterpart to hello_world) | ||
| route "get 'hello_server', to: 'hello_server#index'" | ||
|
|
||
| puts Rainbow("✅ Added RSC routes to config/routes.rb").green | ||
| end |
There was a problem hiding this comment.
Missing guard for absent config/routes.rb.
add_rsc_routes calls File.read(routes_file) on Line 250 without checking whether the file exists, unlike all other methods in this module that guard with File.exist?. If config/routes.rb is missing (unusual but possible), this raises an unhandled Errno::ENOENT.
🛡️ Proposed fix
def add_rsc_routes
routes_file = File.join(destination_root, "config/routes.rb")
+
+ unless File.exist?(routes_file)
+ GeneratorMessages.add_warning(<<~MSG.strip)
+ ⚠️ config/routes.rb not found. Skipping RSC routes.
+
+ You'll need to add the following routes manually:
+ rsc_payload_route
+ get 'hello_server', to: 'hello_server#index'
+ MSG
+ return
+ end
+
routes_content = File.read(routes_file)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def add_rsc_routes | |
| routes_file = File.join(destination_root, "config/routes.rb") | |
| routes_content = File.read(routes_file) | |
| if routes_content.include?("rsc_payload_route") | |
| puts Rainbow("ℹ️ RSC routes already exist, skipping").yellow | |
| return | |
| end | |
| puts Rainbow("📝 Adding RSC routes...").yellow | |
| # Add rsc_payload_route (required for RSC payload requests) | |
| route "rsc_payload_route" | |
| # Add HelloServer route (RSC counterpart to hello_world) | |
| route "get 'hello_server', to: 'hello_server#index'" | |
| puts Rainbow("✅ Added RSC routes to config/routes.rb").green | |
| end | |
| def add_rsc_routes | |
| routes_file = File.join(destination_root, "config/routes.rb") | |
| unless File.exist?(routes_file) | |
| puts Rainbow("⚠️ config/routes.rb not found. Skipping RSC routes.").yellow | |
| puts Rainbow("You'll need to add the following routes manually:").yellow | |
| puts Rainbow(" rsc_payload_route").yellow | |
| puts Rainbow(" get 'hello_server', to: 'hello_server#index'").yellow | |
| return | |
| end | |
| routes_content = File.read(routes_file) | |
| if routes_content.include?("rsc_payload_route") | |
| puts Rainbow("ℹ️ RSC routes already exist, skipping").yellow | |
| return | |
| end | |
| puts Rainbow("📝 Adding RSC routes...").yellow | |
| # Add rsc_payload_route (required for RSC payload requests) | |
| route "rsc_payload_route" | |
| # Add HelloServer route (RSC counterpart to hello_world) | |
| route "get 'hello_server', to: 'hello_server#index'" | |
| puts Rainbow("✅ Added RSC routes to config/routes.rb").green | |
| end |
🤖 Prompt for AI Agents
In `@react_on_rails/lib/generators/react_on_rails/rsc_setup.rb` around lines 248 -
266, The add_rsc_routes method reads config/routes.rb into routes_content
without checking existence, which can raise Errno::ENOENT; guard the read by
checking File.exist?(routes_file) (or early-return with a log) before calling
File.read so add_rsc_routes behaves like the other methods; keep the existing
behavior of skipping when routes already contain "rsc_payload_route" and still
call route "rsc_payload_route" and route "get 'hello_server', to:
'hello_server#index'" only when the file is present.
| // workersCount defaults to the number of CPUs minus 1 | ||
| workersCount: Number(env.NODE_RENDERER_CONCURRENCY) || 3, |
There was a problem hiding this comment.
Number(env) || 3 silently converts 0 to 3, preventing single-process mode via env.
Number("0") || 3 evaluates to 3 because 0 is falsy. If someone sets NODE_RENDERER_CONCURRENCY=0 intending to use single-process mode (as supported by reactOnRailsProNodeRenderer when workersCount === 0), it will be ignored.
Consider using a nullish check instead:
Suggested fix
- workersCount: Number(env.NODE_RENDERER_CONCURRENCY) || 3,
+ workersCount: env.NODE_RENDERER_CONCURRENCY != null ? Number(env.NODE_RENDERER_CONCURRENCY) : 3,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // workersCount defaults to the number of CPUs minus 1 | |
| workersCount: Number(env.NODE_RENDERER_CONCURRENCY) || 3, | |
| // workersCount defaults to the number of CPUs minus 1 | |
| workersCount: env.NODE_RENDERER_CONCURRENCY != null ? Number(env.NODE_RENDERER_CONCURRENCY) : 3, |
🤖 Prompt for AI Agents
In
`@react_on_rails/lib/generators/react_on_rails/templates/pro/base/client/node-renderer.js`
around lines 14 - 15, The current assignment for workersCount uses
Number(env.NODE_RENDERER_CONCURRENCY) || 3 which treats 0 as falsy and overrides
a deliberate "0" setting; change the logic in the workersCount initialization so
a provided "0" is preserved (e.g., use a nullish check or explicit undefined
check), converting the env value to Number only when present: if
env.NODE_RENDERER_CONCURRENCY is null/undefined default to 3, otherwise set
workersCount = Number(env.NODE_RENDERER_CONCURRENCY) (and optionally validate
it’s a safe integer). Ensure this update is applied to the workersCount
expression so single-process mode (0) is honored.
| config.renderer_url = ENV.fetch("REACT_RENDERER_URL", "http://localhost:3800") | ||
|
|
||
| # See value in client/node-renderer.js | ||
| config.renderer_password = ENV.fetch("RENDERER_PASSWORD", "devPassword") |
There was a problem hiding this comment.
Default renderer password applies in all environments including production.
ENV.fetch("RENDERER_PASSWORD", "devPassword") will silently fall back to "devPassword" in production if the environment variable is not set. Consider raising an error in non-development environments when the variable is missing, or at minimum adding a comment warning that this must be set in production.
🛡️ Suggested safer default
- config.renderer_password = ENV.fetch("RENDERER_PASSWORD", "devPassword")
+ # IMPORTANT: Set RENDERER_PASSWORD in production! The default is only for local development.
+ config.renderer_password = ENV.fetch("RENDERER_PASSWORD") { Rails.env.development? ? "devPassword" : raise("RENDERER_PASSWORD env var is required in #{Rails.env}") }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| config.renderer_password = ENV.fetch("RENDERER_PASSWORD", "devPassword") | |
| # IMPORTANT: Set RENDERER_PASSWORD in production! The default is only for local development. | |
| config.renderer_password = ENV.fetch("RENDERER_PASSWORD") { Rails.env.development? ? "devPassword" : raise("RENDERER_PASSWORD env var is required in #{Rails.env}") } |
🤖 Prompt for AI Agents
In
`@react_on_rails/lib/generators/react_on_rails/templates/pro/base/config/initializers/react_on_rails_pro.rb.tt`
at line 10, The current default using ENV.fetch("RENDERER_PASSWORD",
"devPassword") will allow a hardcoded password into production; update the
initializer that sets config.renderer_password so it does not silently default
outside development: retrieve ENV["RENDERER_PASSWORD"] (or ENV.fetch without a
default) and if the value is blank and Rails.env is not development/test raise a
clear error (or abort) explaining RENDERER_PASSWORD must be set; alternatively,
keep the development fallback only when Rails.env.development? and add a comment
above config.renderer_password documenting that the variable is required in
production. Ensure you modify the code path that currently references
config.renderer_password and ENV.fetch("RENDERER_PASSWORD", "devPassword").
There was a problem hiding this comment.
This is by design. The template is a starter configuration for development — users running the node renderer in production must actively configure and deploy it, which involves setting environment variables as part of their deployment process.
The existing manual setup docs in installation.md follow the same pattern (ENV["RENDERER_PASSWORD"] || "changeme"). The node renderer doesn't run by default — it requires explicit setup, a separate Node.js process, and a configured Procfile entry.
Adding a production raise would break the initial rails generate experience (generators load initializers during execution) and would force users to set env vars before they've even finished setup. A comment noting the production requirement is reasonable, but the current pattern is intentional and consistent with the existing documentation.
There was a problem hiding this comment.
@ihabadham, thank you for the context — that makes sense. The generator-loading concern is valid, and consistency with the existing manual setup pattern is important.
If you'd like, a brief inline comment above that line clarifying production requirements would help future readers, but I understand the current design is intentional.
✏️ Learnings added
Learnt from: ihabadham
Repo: shakacode/react_on_rails PR: 2284
File: react_on_rails/lib/generators/react_on_rails/templates/pro/base/config/initializers/react_on_rails_pro.rb.tt:10-10
Timestamp: 2026-02-12T20:43:20.777Z
Learning: In React on Rails Pro initializer templates (e.g., `react_on_rails_pro.rb.tt`), the `config.renderer_password` uses `ENV.fetch("RENDERER_PASSWORD", "devPassword")` as a development starter configuration. This pattern is intentional: the node renderer requires explicit production deployment setup, and adding a production-only raise would break generator execution since initializers are loaded during `rails generate`. The pattern is consistent with existing manual setup documentation.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: react_on_rails_pro/CLAUDE.md:0-0
Timestamp: 2026-02-08T02:13:36.730Z
Learning: Applies to react_on_rails_pro/config/react_on_rails_pro_license.key : License key is configured via `config/react_on_rails_pro_license.key` file or `REACT_ON_RAILS_PRO_LICENSE` environment variable
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: react_on_rails_pro/CLAUDE.md:0-0
Timestamp: 2026-02-08T02:13:36.730Z
Learning: Applies to react_on_rails_pro/package.json : Pro uses `react-server` condition in package.json exports where `import ReactOnRails from 'react-on-rails-pro'` resolves to different files depending on build context (server.ts vs server.rsc.ts)
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: react_on_rails_pro/CLAUDE.md:0-0
Timestamp: 2026-02-08T02:13:36.730Z
Learning: Applies to react_on_rails_pro/lib/react_on_rails_pro/engine.rb : License is checked in Pro engine initializer at `lib/react_on_rails_pro/engine.rb`
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-09T04:51:58.805Z
Learning: Applies to react_on_rails_pro/** : Ask first before modifying the Pro package (`react_on_rails_pro/`)
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: react_on_rails_pro/CLAUDE.md:0-0
Timestamp: 2026-02-08T02:13:36.730Z
Learning: Applies to react_on_rails_pro/**/commonWebpackConfig.js : Changes to `commonWebpackConfig.js` affect all three webpack bundles: client, server, and RSC
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: react_on_rails_pro/CLAUDE.md:0-0
Timestamp: 2026-02-08T02:13:36.730Z
Learning: Applies to react_on_rails_pro/spec/react_on_rails_pro/**/*.rb : Pro gem unit tests should be run with: `cd react_on_rails_pro && bundle exec rspec spec/react_on_rails_pro/`
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: react_on_rails_pro/CLAUDE.md:0-0
Timestamp: 2026-02-08T02:13:36.730Z
Learning: Applies to react_on_rails_pro/**/*.{js,ts,jsx,tsx} : Pro has its own ESLint and Prettier configs separate from root: `eslint.config.mjs`, `.prettierrc`, `.prettierignore`
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1875
File: lib/react_on_rails/utils.rb:112-124
Timestamp: 2025-10-23T17:22:01.074Z
Learning: In React on Rails, when Pro is installed but not licensed, the intended behavior is to raise an error on boot. The `react_on_rails_pro?` method validates licenses and should raise errors early (including during path resolution in methods like `server_bundle?`) to enforce licensing requirements rather than failing later with obscure errors.
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: react_on_rails_pro/CLAUDE.md:0-0
Timestamp: 2026-02-08T02:13:36.730Z
Learning: Applies to react_on_rails_pro/**/*.{js,ts,jsx,tsx} : Pro uses Babel transpiler instead of SWC for advanced transforms
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: react_on_rails_pro/CLAUDE.md:0-0
Timestamp: 2026-02-08T02:13:36.730Z
Learning: Applies to react_on_rails_pro/spec/dummy/spec/**/*.rb : Pro integration tests should be run with: `cd react_on_rails_pro/spec/dummy && bundle exec rspec spec/`
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: react_on_rails_pro/CLAUDE.md:0-0
Timestamp: 2026-02-08T02:13:36.730Z
Learning: Node renderer worker pool defaults to CPU count - 1 and uses JWT-based authentication with password from Rails initializer
Learnt from: ihabadham
Repo: shakacode/react_on_rails PR: 2324
File: react_on_rails_pro/spec/react_on_rails_pro/license_validator_spec.rb:35-45
Timestamp: 2026-01-31T12:34:16.872Z
Learning: In the react_on_rails_pro test environment, the `REACT_ON_RAILS_PRO_LICENSE` environment variable does not exist by default, so restoration logic in RSpec specs is not necessary—simply deleting it in the `after` hook is sufficient.
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1745
File: node_package/src/RSCRequestTracker.ts:8-14
Timestamp: 2025-07-08T05:57:29.630Z
Learning: The global `generateRSCPayload` function in React on Rails Pro (RORP) is provided by the framework during rendering requests, not implemented in application code. The `declare global` statements are used to document the expected interface that RORP will inject at runtime.
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: react_on_rails_pro/CLAUDE.md:0-0
Timestamp: 2026-02-08T02:13:36.730Z
Learning: The node renderer is a standalone Fastify HTTP server that should be started locally with: `cd react_on_rails_pro/spec/dummy && pnpm run node-renderer` (port 3800)
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: react_on_rails_pro/CLAUDE.md:0-0
Timestamp: 2026-02-08T02:13:36.730Z
Learning: Applies to react_on_rails_pro/react_on_rails_pro_node_renderer/package.json : Protocol versioning: `protocolVersion` in package.json must match gem expectations for node renderer
Learnt from: Romex91
Repo: shakacode/react_on_rails PR: 1697
File: package-scripts.yml:28-28
Timestamp: 2025-02-12T16:38:06.537Z
Learning: The file `node_package/lib/ReactOnRails.full.js` is autogenerated during the build process and should not be present in the repository.
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: react_on_rails_pro/CLAUDE.md:0-0
Timestamp: 2026-02-08T02:13:36.730Z
Learning: Applies to react_on_rails_pro/**/*.rsc.{test,spec}.{js,ts,jsx,tsx} : Pro RSC (React Server Components) tests should be run with: `pnpm --filter react-on-rails-pro run test:rsc`
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: react_on_rails_pro/CLAUDE.md:0-0
Timestamp: 2026-02-08T02:13:36.730Z
Learning: Applies to react_on_rails_pro/**/*.{js,ts,jsx,tsx,json,md} : Pro Prettier formatting check should be run with: `cd react_on_rails_pro && pnpm run prettier --check .`
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: react_on_rails_pro/CLAUDE.md:0-0
Timestamp: 2026-02-08T02:13:36.730Z
Learning: Applies to react_on_rails_pro/**/*.{js,ts,jsx,tsx} : Pro ESLint should be run with: `cd react_on_rails_pro && pnpm run eslint .`
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: lib/react_on_rails/helper.rb:190-197
Timestamp: 2025-02-18T13:08:01.477Z
Learning: RSC support validation in React on Rails Pro is handled through a chain of validations:
1. Pro version check in `run_stream_inside_fiber`
2. RSC support check during pack generation via `ReactOnRailsPro.configuration.enable_rsc_support`
3. RSC support validation during component registration
This makes additional validation in the helper methods unnecessary.
| # | ||
| # Uses the same pattern as detecting Rails::Server or Rails::Console context. | ||
| # Rails only loads the Generators module when running generator commands. | ||
| def self.running_generator? | ||
| !ARGV.empty? && ARGV.first&.in?(%w[generate g]) | ||
| defined?(Rails::Generators) | ||
| end |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's run the provided verification script
rg -n "require.*rails/generators" --type=rb -g '!**/generators/**' -g '!**/*_spec.rb'Repository: shakacode/react_on_rails
Length of output: 94
🏁 Script executed:
# Now let's find where running_generator? is actually used
rg -n "running_generator\?" --type=rb -A 3 -B 3Repository: shakacode/react_on_rails
Length of output: 94
🏁 Script executed:
# Let's also check the context around the method and understand what validation it guards
cat -n lib/react_on_rails/engine.rb | sed -n '60,90p'Repository: shakacode/react_on_rails
Length of output: 128
🏁 Script executed:
# Check if there are any requires of rails/generators in the main codebase (not tests/generators)
find lib -name "*.rb" -type f -exec grep -l "require.*rails/generators" {} \;Repository: shakacode/react_on_rails
Length of output: 106
🏁 Script executed:
git ls-files | head -20Repository: shakacode/react_on_rails
Length of output: 863
🏁 Script executed:
# Find engine.rb file
find . -name "engine.rb" -type f 2>/dev/nullRepository: shakacode/react_on_rails
Length of output: 167
🏁 Script executed:
# List the overall directory structure
ls -laRepository: shakacode/react_on_rails
Length of output: 4384
🏁 Script executed:
cat -n ./react_on_rails/lib/react_on_rails/engine.rb | sed -n '60,90p'Repository: shakacode/react_on_rails
Length of output: 1427
🏁 Script executed:
# Search for running_generator? usage
rg "running_generator\?" --context=3Repository: shakacode/react_on_rails
Length of output: 4104
🏁 Script executed:
# Search for requires of rails/generators
rg "require.*rails/generators" --context=2Repository: shakacode/react_on_rails
Length of output: 3223
🏁 Script executed:
# Check if Rails::Generators is referenced elsewhere
rg "Rails::Generators" --context=2Repository: shakacode/react_on_rails
Length of output: 20083
The comment overstates Rails's guarantee about when Rails::Generators is defined; the fallback nature limits risk but doesn't eliminate it.
The stated assumption that "Rails only loads the Generators module when running generator commands" isn't strictly guaranteed. Rails::Generators can be defined whenever any code calls require "rails/generators" — including test helpers (as seen in spec/react_on_rails/support/generator_spec_helper.rb) or any gem initializer.
Currently, all such requires are isolated to the generator files themselves, and since running_generator? is a fallback behind the ENV check, the practical blast radius is limited. However, if a gem or initializer ever requires generators, validation would be silently skipped when package.json exists and ENV isn't set. Consider clarifying the comment to reflect the actual limitation rather than implying Rails provides a hard guarantee.
🤖 Prompt for AI Agents
In `@react_on_rails/lib/react_on_rails/engine.rb` around lines 72 - 77, The
comment above the self.running_generator? method overstates Rails' guarantee;
update the comment to explain that the check (defined?(Rails::Generators)) is a
heuristic/fallback rather than a strict guarantee — Rails::Generators can be
defined whenever code calls require "rails/generators" (e.g., test helpers or
gems/initializers), so running_generator? may be true outside of generator
commands; keep mention that this is behind the ENV check and is limited in
practice but clarify the limitation and potential false positives instead of
implying Rails only loads the module during generator execution.
| dev-server: bin/shakapacker-dev-server | ||
| server-bundle: SERVER_BUNDLE_ONLY=true bin/shakapacker --watch |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for SERVER_BUNDLE_ONLY usage in the codebase
rg 'SERVER_BUNDLE_ONLY' -B 2 -A 2 --type ruby --type shell --type textRepository: shakacode/react_on_rails
Length of output: 97
🏁 Script executed:
#!/bin/bash
# List the Procfile.dev files to confirm they exist
find . -name "Procfile.dev" -type f | head -20Repository: shakacode/react_on_rails
Length of output: 296
🏁 Script executed:
#!/bin/bash
# Check the actual content of both Procfile.dev files
echo "=== spec/dummy/Procfile.dev ===" && cat spec/dummy/Procfile.dev 2>/dev/null || echo "File not found"
echo ""
echo "=== Template Procfile.dev ===" && find . -path "*/templates/base/base/Procfile.dev" -exec cat {} \; || echo "Template file not found"Repository: shakacode/react_on_rails
Length of output: 356
🏁 Script executed:
#!/bin/bash
# Try with proper path - the repo is in react_on_rails subdirectory
cat react_on_rails/spec/dummy/Procfile.devRepository: shakacode/react_on_rails
Length of output: 343
🏁 Script executed:
#!/bin/bash
# Search for SERVER_BUNDLE_ONLY in Ruby/shell files without shell type restriction
rg 'SERVER_BUNDLE_ONLY' -B 3 -A 3Repository: shakacode/react_on_rails
Length of output: 8175
Inconsistency: SERVER_BUNDLE_ONLY=true vs template's =yes.
The template at lib/generators/react_on_rails/templates/base/base/Procfile.dev uses SERVER_BUNDLE_ONLY=yes, while this dummy Procfile uses true. In JavaScript, both evaluate as truthy and work equivalently, but the template and most other Procfiles in the codebase use yes. Align this to yes for consistency.
🤖 Prompt for AI Agents
In `@react_on_rails/spec/dummy/Procfile.dev` around lines 4 - 5, Replace the
inconsistent boolean value in the Procfile.dev entry so it matches the project
template: update the server-bundle line that currently sets
SERVER_BUNDLE_ONLY=true to use SERVER_BUNDLE_ONLY=yes; this change is in the
Procfile.dev “server-bundle” entry and will align it with the template at
lib/generators/react_on_rails/templates/base/base/Procfile.dev.
|
|
||
| # Pro/RSC prerequisite validation tests | ||
|
|
||
| context "when using --pro flag without Pro gem installed" do | ||
| let(:install_generator) { described_class.new([], { pro: true }) } | ||
|
|
||
| specify "missing_pro_gem? returns true and error mentions --pro flag" do | ||
| allow(Gem).to receive(:loaded_specs).and_return({}) | ||
| allow(install_generator).to receive(:gem_in_lockfile?).with("react_on_rails_pro").and_return(false) | ||
|
|
||
| expect(install_generator.send(:missing_pro_gem?)).to be true | ||
| error_text = GeneratorMessages.messages.join("\n") | ||
| expect(error_text).to include("--pro") | ||
| expect(error_text).to include("react_on_rails_pro") | ||
| expect(error_text).to include("Try Pro free!") | ||
| end | ||
| end | ||
|
|
||
| context "when using --rsc flag without Pro gem installed" do | ||
| let(:install_generator) { described_class.new([], { rsc: true }) } | ||
|
|
||
| specify "missing_pro_gem? returns true and error mentions --rsc flag" do | ||
| allow(Gem).to receive(:loaded_specs).and_return({}) | ||
| allow(install_generator).to receive(:gem_in_lockfile?).with("react_on_rails_pro").and_return(false) | ||
|
|
||
| expect(install_generator.send(:missing_pro_gem?)).to be true | ||
| error_text = GeneratorMessages.messages.join("\n") | ||
| expect(error_text).to include("--rsc") | ||
| end | ||
| end | ||
|
|
||
| context "when using --pro flag with Pro gem in Gem.loaded_specs" do | ||
| let(:install_generator) { described_class.new([], { pro: true }) } | ||
|
|
||
| specify "missing_pro_gem? returns false" do | ||
| allow(Gem).to receive(:loaded_specs).and_return({ "react_on_rails_pro" => double }) | ||
|
|
||
| expect(install_generator.send(:missing_pro_gem?)).to be false | ||
| end | ||
| end | ||
|
|
||
| context "when using --pro flag with Pro gem in Gemfile.lock" do | ||
| let(:install_generator) { described_class.new([], { pro: true }) } | ||
|
|
||
| specify "missing_pro_gem? returns false" do | ||
| allow(Gem).to receive(:loaded_specs).and_return({}) | ||
| allow(install_generator).to receive(:gem_in_lockfile?).with("react_on_rails_pro").and_return(true) | ||
|
|
||
| expect(install_generator.send(:missing_pro_gem?)).to be false | ||
| end | ||
| end | ||
|
|
||
| context "when not using --pro or --rsc flags" do | ||
| let(:install_generator) { described_class.new } | ||
|
|
||
| specify "missing_pro_gem? returns false without checking gem" do | ||
| expect(install_generator.send(:missing_pro_gem?)).to be false | ||
| end | ||
| end |
There was a problem hiding this comment.
Consider clearing GeneratorMessages in prerequisite validation tests.
These unit-style test contexts (lines 1072–1127) call methods like missing_pro_gem? that append to GeneratorMessages, but don't clear the messages beforehand. Since GeneratorMessages holds class-level state, messages from earlier examples can accumulate and leak across contexts. While current assertions use include matchers that tolerate extra content, this could mask issues or cause confusing failures if assertions become more precise later.
Consider adding GeneratorMessages.clear in the before block for each context, consistent with the pattern on line 916.
🤖 Prompt for AI Agents
In `@react_on_rails/spec/react_on_rails/generators/install_generator_spec.rb`
around lines 1069 - 1127, Add a before hook in each of the four prerequisite
validation contexts to call GeneratorMessages.clear so tests don't leak
class-level messages between examples: for the contexts that define
let(:install_generator) with flags (pro/rsc) and the one without flags, add
before { GeneratorMessages.clear } above the existing stubs/allow calls so
missing_pro_gem? and subsequent assertions operate on a clean message state;
reference the contexts using the install_generator let and the missing_pro_gem?
method and ensure the before hook is placed consistently as done around line 916
in other specs.
1 participant
Summary
Adds
--proand--rscflags to the existingrails g react_on_rails:installgenerator.New commands:
Closes #2277
Current Status
Phase 1: Foundation ✅
--proand--rscclass optionsuse_pro?,use_rsc?,pro_gem_installed?)Phase 2: Pro Generator ✅
config/initializers/react_on_rails_pro.rbwith Node Renderer configclient/node-renderer.jsbootstrap fileProcfile.devreact-on-rails-proandreact-on-rails-pro-node-renderernpm packagesreact-on-railspackage when installing Pro)Phase 3: RSC Generator 🔜
Phase 4: Testing (In Progress)
--proflag--pro+ other flags combinations--rscflagPhase 5: Documentation 🔜
What's Implemented
Pro Generator (
--pro)When running
rails g react_on_rails:install --pro:Pro Initializer - Creates
config/initializers/react_on_rails_pro.rbwith:--rscis used)Node Renderer - Creates
client/node-renderer.jswith:Procfile.dev - Appends node-renderer process
NPM Packages - Installs Pro dependencies
Idempotency
The generator is idempotent:
Flag Combinations
Works with existing flags:
--pro --typescript--pro --redux--pro --rspackTest Plan
bundle exec rspec spec/react_on_rails/generators/install_generator_spec.rb- all tests pass🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
--proand--rscgenerator options for React on Rails Pro and React Server Components setupDocumentation