Improve SWC compiler detection and default dependencies #2135
Conversation
Add using_swc? helper method to detect SWC configuration: - Parses shakapacker.yml for javascript_transpiler setting - Returns true when SWC is explicitly configured - Returns true for Shakapacker 9.3.0+ when not specified (SWC default) - Returns true for fresh installations (SWC is recommended) Add SWC dependency installation to generator: - Add SWC_DEPENDENCIES constant (@swc/core, swc-loader) - Install SWC deps automatically when using_swc? returns true - Add to devDependencies since they're build-time tools Update spec/dummy to include SWC packages: - Add @swc/core and swc-loader to devDependencies - Matches the javascript_transpiler: swc setting in shakapacker.yml Add comprehensive tests for using_swc? helper: - Test explicit swc config returns true - Test explicit babel config returns false - Test missing config defaults to SWC for 9.3.0+ - Test missing file defaults to SWC - Test invalid YAML defaults to SWC Closes #1956 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
WalkthroughIntroduces SWC configuration detection in the generator helper with automatic Shakapacker version checking and shakapacker.yml parsing. Adds conditional SWC dependency installation in the JS dependency manager. Includes comprehensive tests for the new SWC detection logic. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (6)
🧰 Additional context used📓 Path-based instructions (2)**/*.{rb,rake}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (14)📚 Learning: 2025-11-25T08:05:17.804ZApplied to files:
📚 Learning: 2025-02-18T13:08:01.477ZApplied to files:
📚 Learning: 2025-11-25T08:05:17.804ZApplied to files:
📚 Learning: 2025-02-13T14:29:49.267ZApplied to files:
📚 Learning: 2025-09-29T14:45:42.687ZApplied to files:
📚 Learning: 2025-02-12T16:38:06.537ZApplied to files:
📚 Learning: 2025-11-25T08:05:17.804ZApplied to files:
📚 Learning: 2025-09-16T08:01:11.146ZApplied to files:
📚 Learning: 2025-04-26T21:55:55.874ZApplied to files:
📚 Learning: 2025-01-23T18:20:45.824ZApplied to files:
📚 Learning: 2025-11-25T08:05:17.804ZApplied to files:
📚 Learning: 2024-07-27T10:08:35.868ZApplied to files:
📚 Learning: 2025-11-25T08:05:17.804ZApplied to files:
📚 Learning: 2025-07-08T05:57:29.630ZApplied to files:
🧬 Code graph analysis (3)react_on_rails/spec/react_on_rails/generators/generator_helper_spec.rb (2)
react_on_rails/spec/react_on_rails/generators/install_generator_spec.rb (1)
react_on_rails/lib/generators/react_on_rails/js_dependency_manager.rb (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
🔇 Additional comments (14)
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: PR #2135 - Improve SWC compiler detection and default dependenciesSummaryThis PR adds intelligent SWC (Speedy Web Compiler) detection and automatic dependency installation for the React on Rails generator. Overall, the implementation is well-designed and production-ready with excellent test coverage and error handling. ✅ Strengths1. Excellent Error Handling & Graceful DegradationThe code follows a defensive programming approach that aligns well with the project's error handling philosophy: def parse_shakapacker_yml(path)
require "yaml"
YAML.load_file(path, aliases: true)
rescue ArgumentError
# Older Psych versions don't support the aliases parameter
YAML.load_file(path)
rescue StandardError
# If we can't parse the file, return empty config
{}
end
2. Smart Default BehaviorThe detection logic makes sensible assumptions:
3. Comprehensive Test CoverageThe specs cover all edge cases: ✅ Explicit SWC configuration
✅ Explicit Babel configuration
✅ No transpiler specified (version-based default)
✅ Missing shakapacker.yml
✅ Malformed YAML parsing errors4. Proper Memoizationdef using_swc?
return @using_swc if defined?(@using_swc)
@using_swc = detect_swc_configuration
endUses 5. Clear DocumentationExcellent inline documentation explaining:
🔍 Potential Issues & Suggestions1. Missing Version Check Implementation
|
| Category | Rating | Notes |
|---|---|---|
| Error Handling | ⭐⭐⭐⭐⭐ | Excellent graceful degradation |
| Test Coverage | ⭐⭐⭐⭐½ | Comprehensive, missing version boundary test |
| Documentation | ⭐⭐⭐⭐⭐ | Clear comments and inline docs |
| Code Style | ⭐⭐⭐⭐⭐ | Follows project conventions |
| Security | ⭐⭐⭐⭐☆ | Minor YAML.load concern (low risk) |
| Performance | ⭐⭐⭐⭐⭐ | Proper memoization |
🚀 Testing Recommendations
Before merging, please verify:
-
Fresh Installation Test:
# In a new Rails app without Shakapacker rails new test_app cd test_app bundle add react_on_rails --git=<your-branch> rails generate react_on_rails:install # Verify SWC dependencies are installed grep -E '@swc/core|swc-loader' package.json
-
Babel Override Test:
# Edit config/shakapacker.yml BEFORE running generator # Set javascript_transpiler: babel rails generate react_on_rails:install # Verify SWC dependencies are NOT installed grep -E '@swc/core|swc-loader' package.json # Should be empty
-
Existing Installation Test:
# In an app with existing Shakapacker 9.3.0+ rails generate react_on_rails:install # Verify SWC deps are added
📋 Checklist for Author
- Manual testing completed per recommendations above
- CI passes (RuboCop, RSpec)
- Tested with both fresh and existing installations
- Verified SWC deps are correctly installed/skipped based on config
- Consider adding version boundary test (optional)
- Consider YAML.safe_load_file (optional security hardening)
✅ Final Recommendation
APPROVE with minor suggestions.
This is a well-implemented feature that:
- Solves a real problem (manual SWC dependency installation)
- Has excellent error handling
- Includes comprehensive tests
- Follows project conventions
- Makes sensible default assumptions
The suggestions above are minor improvements, not blocking issues. The code is production-ready as-is.
Great work! 🎉
Reviewed by: Claude Code
Date: 2025-11-26
The JsDependencyManager tests create a mock test class that includes the module. After adding the using_swc? call to add_js_dependencies, the test class needs to provide this method (which comes from GeneratorHelper in actual generator classes). Changes: - Add using_swc? method stub to test class (defaults to true) - Add using_swc setter for test control - Add test for SWC_DEPENDENCIES constant 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Changes based on code review feedback: 1. Security: Use YAML.safe_load_file instead of YAML.load_file - Defense-in-depth even though it's the user's own config - Added fallback chain for older Psych versions 2. Documentation: Add caching behavior note to using_swc? - Explains memoization is per-generator-instance - Notes config changes during execution won't be reflected 3. Tests: Add version boundary tests for Shakapacker 9.3.0 - Test SWC default when version is 9.3.0+ - Test Babel default when version is below 9.3.0 - Test graceful fallback when version check fails 4. Tests: Add generator test verifying SWC dependency installation - Confirms @swc/core and swc-loader in devDependencies - Mirrors existing rspack dependency test pattern 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Summary
using_swc?helper method to detect SWC configuration in shakapacker.ymlKey Changes
1. Enhanced
using_swc?Helpershakapacker.ymlforjavascript_transpilersettingtruewhen SWC is explicitly configuredtruefor Shakapacker 9.3.0+ when not specified (SWC is the default)truefor fresh installations (SWC is recommended)2. Generator Updates
SWC_DEPENDENCIESconstant with @swc/core and swc-loaderusing_swc?returns true3. spec/dummy Updates
javascript_transpiler: swcsetting already in shakapacker.ymlTesting
using_swc?helperTest Plan
Closes #1956
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.