Improve SWC compiler detection and default dependencies

[justin808]

Summary

• Add using_swc? helper method to detect SWC configuration in shakapacker.yml
• Automatically install SWC dependencies (@swc/core, swc-loader) when SWC is configured
• Update spec/dummy to include SWC packages

Key Changes

1. Enhanced using_swc? Helper

• 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 is the default)
• Returns true for fresh installations (SWC is recommended)

2. Generator Updates

• Add SWC_DEPENDENCIES constant with @swc/core and swc-loader
• Install SWC deps automatically when using_swc? returns true
• Add to devDependencies since they're build-time tools

3. spec/dummy Updates

• Add @swc/core and swc-loader to devDependencies
• Matches the javascript_transpiler: swc setting already in shakapacker.yml

Testing

• Added comprehensive RSpec tests for using_swc? helper
• All existing tests pass
• RuboCop checks pass

Test Plan

• Test generator with Shakapacker 9.3.0+
• Verify SWC detection logic
• Test fallback to Babel when explicitly configured

Closes #1956

🤖 Generated with Claude Code

[coderabbitai]

Warning

Rate limit exceeded

@justin808 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 19 minutes and 46 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 16b3908 and 7be99ea.

📒 Files selected for processing (5)

• react_on_rails/lib/generators/react_on_rails/generator_helper.rb (1 hunks)
• react_on_rails/lib/generators/react_on_rails/js_dependency_manager.rb (3 hunks)
• react_on_rails/spec/dummy/package.json (2 hunks)
• react_on_rails/spec/react_on_rails/generators/generator_helper_spec.rb (1 hunks)
• react_on_rails/spec/react_on_rails/generators/js_dependency_manager_spec.rb (2 hunks)

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch jg/swc-detection-fix

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[claude]

Code Review: PR #2135 - Improve SWC compiler detection and default dependencies

Summary

This 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.

---

✅ Strengths

1. Excellent Error Handling & Graceful Degradation

The 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

• ✅ Handles both old and new Psych versions
• ✅ Returns empty config instead of raising exceptions
• ✅ Prevents generator crashes from malformed YAML files

2. Smart Default Behavior

The detection logic makes sensible assumptions:

• Explicitly configured transpilers take precedence (babel vs swc)
• Defaults to SWC for Shakapacker 9.3.0+ (matches upstream default)
• Assumes latest version for fresh installs (optimistic but safe)
• Falls back gracefully when version detection fails

3. Comprehensive Test Coverage

The specs cover all edge cases:

✅ Explicit SWC configuration
✅ Explicit Babel configuration  
✅ No transpiler specified (version-based default)
✅ Missing shakapacker.yml
✅ Malformed YAML parsing errors

4. Proper Memoization

def using_swc?
  return @using_swc if defined?(@using_swc)
  @using_swc = detect_swc_configuration
end

Uses defined?(@using_swc) instead of just checking truthiness, which correctly handles false values.

5. Clear Documentation

Excellent inline documentation explaining:

• What the method does
• Detection logic flow
• Why certain defaults are chosen
• Performance benefits ("20x faster than Babel")

---

🔍 Potential Issues & Suggestions

1. Missing Version Check Implementation ⚠️

The code calls shakapacker_version_9_3_or_higher? but I notice it's a new method. Let me verify the implementation:

def shakapacker_version_9_3_or_higher?
  return true unless defined?(ReactOnRails::PackerUtils)
  
  ReactOnRails::PackerUtils.shakapacker_version_requirement_met?("9.3.0")
rescue StandardError
  # If we can't determine version, assume latest (which uses SWC)
  true
end

Good: This follows the same pattern as the existing shakapacker_version_9_or_higher? method.

Concern: The method returns true when ReactOnRails::PackerUtils is not defined. During generator runtime, this might not be defined yet.

Recommendation: Verify this works correctly during fresh installations where Shakapacker hasn't been installed yet. The optimistic default is probably correct, but worth confirming.

2. Dependency Installation Ordering 💡

In js_dependency_manager.rb:

def add_js_dependencies
  add_react_on_rails_package
  add_react_dependencies
  add_css_dependencies
  add_rspack_dependencies if respond_to?(:options) && options&.rspack?
  add_swc_dependencies if using_swc?  # ← Called here
  add_dev_dependencies
end

Question: Should SWC dependencies be added earlier in the sequence, perhaps before CSS dependencies? Build tools typically need to be installed before other dependencies that might use them.

Recommendation: Consider moving add_swc_dependencies right after add_react_on_rails_package or at least before add_css_dependencies.

3. YAML.load_file Security 🔒

YAML.load_file(path, aliases: true)

Current: Uses YAML.load_file which can execute arbitrary Ruby code in older YAML versions.

Recommendation: Consider using YAML.safe_load_file for better security:

def parse_shakapacker_yml(path)
  require "yaml"
  YAML.safe_load_file(path, aliases: true, permitted_classes: [Symbol])
rescue ArgumentError
  YAML.safe_load_file(path, permitted_classes: [Symbol])
rescue StandardError
  {}
end

However: Since this is reading the user's own shakapacker.yml config file (not external input), the security risk is minimal. But it's worth considering for defense-in-depth.

4. Missing Test: Version Boundary 📝

The tests cover the logic well, but there's no test for the version boundary:

Suggestion: Add a test that mocks ReactOnRails::PackerUtils.shakapacker_version_requirement_met? to return both true and false for version 9.3.0.

Example:

context "when Shakapacker version is 9.2.0 (before SWC default)" do
  before do
    allow(ReactOnRails::PackerUtils).to receive(:shakapacker_version_requirement_met?)
      .with("9.3.0").and_return(false)
  end

  it "returns false when no transpiler is specified" do
    File.write(shakapacker_yml_path, "default: {}\n")
    expect(using_swc?).to be false
  end
end

5. Potential Race Condition with Memoization 🧵

The memoization uses instance variables:

def using_swc?
  return @using_swc if defined?(@using_swc)
  @using_swc = detect_swc_configuration
end

Concern: If shakapacker.yml changes during generator execution, the memoized value won't update.

Reality Check: This is probably fine since generators run quickly and config files shouldn't change mid-execution.

Optional Enhancement: Add a comment explaining this caching behavior.

---

🎯 Code Quality Assessment

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:

1. 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

2. 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

3. 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