PR Review: Upgrade to Shakapacker 9.0.0.beta.11 and migrate to SWC

✅ Positive Aspects

Good migration approach: Using the official Shakapacker migration tasks (shakapacker:migrate_to_swc and shakapacker:clean_babel_packages) is the correct approach and reduces the risk of manual errors.

Performance benefits: The 20x performance improvement from SWC is significant and will benefit both development and CI workflows.

Clean dependency removal: All Babel-related packages have been properly removed without leaving orphaned dependencies.

Proper SWC configuration: The swc.config.js uses React automatic runtime, which is correct for modern React.

🔍 Issues & Concerns

1. CRITICAL: Version Inconsistency in package.json

Location: spec/dummy/package.json:48

The package.json still shows:

"shakapacker": "8.2.0",

But should be updated to 9.0.0.beta.11 to match the Gemfile changes. The yarn.lock appears to be updated, but package.json is not. This could cause confusion and potential issues with tooling that reads package.json directly.

Recommendation: Update line 48 in spec/dummy/package.json to:

"shakapacker": "9.0.0.beta.11",

2. Beta Version in Production

Using a beta version (9.0.0.beta.11) poses risks:

• Potential undiscovered bugs
• Breaking changes before stable release
• Limited community testing
• May require migration work when upgrading to stable

Recommendation:

• Ensure comprehensive testing before merging
• Document the beta status in CHANGELOG/README
• Plan to upgrade to stable 9.0.0 when released
• Monitor Shakapacker releases closely

3. Missing Migration Documentation

For users upgrading React on Rails, there's no clear documentation about:

• Required steps to migrate to this version
• Breaking changes from the Shakapacker upgrade
• How to handle the Babel → SWC migration in their apps

Recommendation: Add upgrade notes to the CHANGELOG or migration guide.

4. Potential React Refresh Compatibility

The PR removes Babel but keeps @pmmmwh/react-refresh-webpack-plugin and react-refresh. Need to verify these work correctly with SWC.

Recommendation:

• Test that hot module replacement still works in development
• Verify React Fast Refresh functions correctly with SWC

5. YAML Configuration Duplication

Location: spec/dummy/config/shakapacker.yml

The YAML configuration has significant duplication across environments (development, test, production). While this is how the migration tool generated it, it reduces maintainability.

Observation: This appears to be the standard Shakapacker 9 format (removed YAML anchors/aliases except for additional_paths). If this is intentional, it's fine but makes updates more error-prone.

🧪 Testing Recommendations

Before merging, verify:

1. Build successful in all environments:

   RAILS_ENV=development yarn build:dev
   RAILS_ENV=test yarn build:test
   RAILS_ENV=production NODE_ENV=production bin/shakapacker

2. HMR/Fast Refresh works: Start dev server and verify hot reloading works

3. Server-side rendering: Verify SSR still works correctly (critical for react_on_rails)

4. Example apps: Run all example app tests

   rake run_rspec:example_basic
   # Test other examples too

5. Type checking: Ensure TypeScript compilation works with SWC

6. Bundle size: Compare production bundle sizes before/after

🔒 Security Considerations

No security concerns identified. The upgrade removes dependencies, reducing attack surface.

📊 Test Coverage

Test plan incomplete: The PR description shows "CI checks passing" is unchecked. This must be completed before merge.

Recommendation:

• Wait for all CI checks to pass
• Run full test suite locally
• Test with all supported Ruby/Node versions

📝 Code Quality

Code quality: ✅ Good

• Configuration follows SWC and Shakapacker best practices
• Proper React automatic runtime configuration
• Clean removal of deprecated code

Convention compliance: ✅ Follows CLAUDE.md requirements

• Would need to run bundle exec rubocop before commit (per CLAUDE.md)
• Prettier formatting should be applied via rake autofix

🎯 Summary

Recommendation: APPROVE with required changes

Must fix before merge:

1. ❌ Update spec/dummy/package.json line 48 to use version 9.0.0.beta.11
2. ❌ Ensure all CI checks pass
3. ❌ Test React Fast Refresh/HMR functionality

Should consider:

• Document beta version usage and upgrade plan
• Add migration guide for users
• Verify all example apps work correctly

The migration is well-executed overall, but the package.json version inconsistency must be fixed, and thorough testing is required before merging a beta version dependency.

PR Review: Upgrade to Shakapacker 9.0.0.beta.11 and migrate to SWC

✅ Overall Assessment

This is a well-executed migration that brings significant performance improvements. The changes are clean, focused, and follow the Shakapacker migration guidelines. However, there are a few items that need attention before merging.

🔴 Critical Issues

1. @babel/core Still Present in Dependencies

The package still has @babel/core@7.17.9 in devDependencies (spec/dummy/package.json:31). This should be removed as part of the Babel cleanup since SWC replaces Babel entirely.

"@babel/core": "7.17.9", // ← Should be removed

Why it matters: Keeping unused dependencies increases bundle size and can cause confusion about which transpiler is being used.

Recommendation: Remove this from spec/dummy/package.json and run yarn install to update the lockfile.

⚠️ Important Considerations

2. React Refresh Configuration

The code still references process.env.WEBPACK_SERVE in the deleted babel.config.js, but I don't see equivalent configuration in swc.config.js for React Refresh integration. The development.js webpack config correctly uses @pmmmwh/react-refresh-webpack-plugin, but SWC needs to be configured to work with it.

Current SWC config:

module.exports = {
 jsc: {
 transform: {
 react: {
 runtime: 'automatic',
 },
 },
 },
};

Recommended addition (if not handled by Shakapacker defaults):

module.exports = {
 jsc: {
 transform: {
 react: {
 runtime: 'automatic',
 development: process.env.NODE_ENV === 'development',
 refresh: process.env.NODE_ENV === 'development', // Enable Fast Refresh
 },
 },
 },
};

Action needed: Verify that React Fast Refresh works correctly in development mode. If not, add the refresh and development options to swc.config.js.

3. Beta Version in Production-Ready Project

Using shakapacker@9.0.0.beta.11 means this project will depend on a beta version.

Questions to consider:

• Is there a timeline for Shakapacker 9.0 stable release?
• Are there any known issues in this beta that might affect users?
• Should this be documented in the changelog/release notes?

Recommendation: Add a note in the PR description or in project documentation about the beta dependency and any implications for users.

💡 Code Quality & Best Practices

4. SWC Configuration Documentation

The swc.config.js file has good comments, but consider adding:

• A comment about what specific transformations are being applied
• Migration notes for developers who might need to customize SWC settings

Example enhancement:

// config/swc.config.js
// This file is merged with Shakapacker's default SWC configuration
// See: https://swc.rs/docs/configuration/compilation
//
// Key differences from Babel:
// - Uses 'automatic' runtime (no need to import React in JSX files)
// - SWC handles modern JS features out of the box
// - For custom transforms, see SWC plugin documentation
module.exports = {
 jsc: {
 transform: {
 react: {
 runtime: 'automatic', // Enables new JSX transform (React 17+)
 },
 },
 },
};

5. YAML Formatting in shakapacker.yml

The YAML changes show significant reformatting (anchors &1, removal of comments). While this is likely from running the migration task, it reduces readability.

Before (comments provided context):

# Additional paths webpack should lookup modules
# ['app/assets', 'engine/foo/app/assets']
additional_paths: []

After:

additional_paths: &1 []

Recommendation: Consider restoring some of the helpful comments in shakapacker.yml to maintain developer-friendliness. The comments about HMR, compression, and compilation behavior were valuable.

✅ What's Done Well

1. Clean dependency removal: All Babel packages properly removed except @babel/core (see issue TODO for first version #1 )
2. Proper SWC package additions: @swc/core and swc-loader correctly added
3. Configuration consistency: swc: true added to all environments (dev/test/prod)
4. Following migration process: Used official Shakapacker rake tasks for migration
5. Performance benefits: ~20x faster builds is a huge win
6. Minimal diff: Changes are focused and don't introduce unnecessary modifications

🧪 Testing Recommendations

Before merging, please verify:

• ✅ Development server starts correctly with HMR
• ✅ React Fast Refresh works (edit a component and verify hot reload without full page refresh)
• ✅ Production build completes successfully
• ✅ Test builds work correctly
• ✅ Server-side rendering still functions
• ✅ All example apps build and run
• ✅ TypeScript compilation works (if applicable)
• ✅ Source maps are generated correctly for debugging

📋 Action Items Summary

1. Required before merge:

   • Remove @babel/core from spec/dummy/package.json
   • Verify React Fast Refresh functionality, add SWC config if needed
   • All CI checks must pass

2. Recommended:

   • Add helpful comments back to shakapacker.yml
   • Document beta version dependency implications
   • Enhance swc.config.js comments for future maintainers

🎯 Verdict

This is a solid migration that will significantly improve build performance. With the @babel/core dependency removed and React Refresh verified, this should be ready to merge.

Great work on following the migration guidelines and keeping the changes focused! The performance improvements will be a huge benefit to developers using this gem.

🤖 Generated with Claude Code

Code Review: Shakapacker 9.0.0.beta.11 + SWC Migration

I've reviewed this pull request and here's my feedback:

Strengths

1. Clean Migration: The transition from Babel to SWC is well-executed with proper dependency management - all Babel packages correctly removed, SWC dependencies properly added, no orphaned dependencies

2. Configuration Quality: The new swc.config.js is well-documented with clear comments and proper React automatic runtime configuration

3. Shakapacker.yml Updates: Clean addition of swc: true flag with good inline comments

Observations & Considerations

Beta Version Risk: Using 9.0.0.beta.11 in a library carries risk. Consider adding release notes warning.

Missing PropTypes Optimization: The removed babel-plugin-transform-react-remove-prop-types was stripping PropTypes in production. With SWC, this optimization is lost, leading to slightly larger bundle sizes. Consider adding SWC equivalent transform or documenting this trade-off.

Testing Coverage: PR mentions linters passing but no mention of actual functionality testing (SSR, client rendering). Recommend running full test suite.

Breaking Change Documentation: This is significant toolchain change. Consider adding migration guide for users.

Potential Issues

1. PropTypes won't be stripped in production anymore (bundle size impact)
2. Verify SWC generates source maps correctly
3. Test that Fast Refresh works with WEBPACK_SERVE conditional

Security & Performance

• No security concerns identified
• Major performance win: 20x faster transpilation
• Should significantly improve CI build times
• Monitor production bundle sizes

Recommendations Before Merge

1. Run full test suite including rake run_rspec, yarn run test, and example apps
2. Address PropTypes handling (configure SWC transform or document trade-off)
3. Update CHANGELOG.md with breaking change notice and migration instructions
4. Wait for CI to pass (test plan shows this unchecked)

Code Quality Score: 8.5/10

Overall: High-quality migration with significant build performance improvements. Main concerns are beta stability and PropTypes optimization loss. Once CI passes and recommendations addressed, should be good to merge.

Great work on the clean migration!

Generated with Claude Code