Add assets_bundler to config if missing during bundler switch (#833)

## Summary
- Fixes an issue where `rake shakapacker:switch_bundler rspack
--install-deps` silently fails when `assets_bundler` is missing from
`shakapacker.yml`
- Automatically adds the `assets_bundler` key with appropriate comments
when missing
- Prevents early exit when the key is missing but the default value
matches the target bundler

## Key Changes
1. **Detection**: Check if `assets_bundler` key exists before early exit
logic
2. **Addition**: Add the key intelligently based on config structure:
   - After `javascript_transpiler` if present
   - After `source_path` if `javascript_transpiler` missing
   - After `default: &default` as last resort
3. **Preservation**: Maintain existing behavior for configs that already
have the key

## Test Plan
- Added test for adding `assets_bundler` after `javascript_transpiler`
- Added test for adding `assets_bundler` when `javascript_transpiler` is
missing
- All existing tests pass
- RuboCop passes

## Impact
This fix is particularly helpful for users migrating from older
Shakapacker versions where `assets_bundler` wasn't part of the default
config.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **New Features**
* Enhanced bundler switching with configuration validation to ensure
correct assets_bundler setup when switching between rspack and webpack.
* Improved configuration file handling with better placement logic and
comment preservation.

* **Bug Fixes**
* Fixed handling of commented-out configuration entries during bundler
switches.
* Enhanced edge-case support for non-standard configuration structures.

* **Tests**
* Added comprehensive test coverage for bundler switching scenarios and
configuration updates.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Claude <noreply@anthropic.com>

Author: justin808

lib/shakapacker/bundler_switcher.rb

@@ -9,6 +9,9 @@ class BundlerSwitcher
     SHAKAPACKER_CONFIG = "config/shakapacker.yml"
     CUSTOM_DEPS_CONFIG = ".shakapacker-switch-bundler-dependencies.yml"
 
+    # Regex pattern to detect assets_bundler key in config (only matches uncommented lines)
+    ASSETS_BUNDLER_PATTERN = /^[ \t]*assets_bundler:/
+
     # Default dependencies for each bundler (package names only, no versions)
     DEFAULT_RSPACK_DEPS = {
       dev: %w[@rspack/cli @rspack/plugin-react-refresh],
@@ -37,18 +40,33 @@ def switch_to(bundler, install_deps: false, no_uninstall: false)
       end
 
       current = current_bundler
-      if current == bundler && !install_deps
+      config_content = File.read(config_path)
+      has_assets_bundler = config_content =~ ASSETS_BUNDLER_PATTERN
+
+      # Early exit if already using the target bundler
+      # For webpack: if current is webpack, we're done (key optional due to default)
+      # For rspack: requires explicit key to be present
+      already_configured = if bundler == "webpack"
+        current == bundler
+      else
+        current == bundler && has_assets_bundler
+      end
+
+      if already_configured && !install_deps
         puts "✅ Already using #{bundler}"
         return
       end
 
-      if current == bundler && install_deps
+      if already_configured && install_deps
         puts "✅ Already using #{bundler} - reinstalling dependencies as requested"
         manage_dependencies(bundler, install_deps, switching: false, no_uninstall: no_uninstall)
         return
       end
 
-      update_config(bundler)
+      successfully_updated = update_config(bundler, config_content, has_assets_bundler)
+
+      # Verify the update was successful (only if update reported success)
+      verify_config_update(bundler) if successfully_updated
 
       puts "✅ Switched from #{current} to #{bundler}"
       puts ""
@@ -143,20 +161,74 @@ def load_dependencies
         end
       end
 
-      def update_config(bundler)
-        content = File.read(config_path)
+      def update_config(bundler, content, has_assets_bundler)
+        # Check if assets_bundler key exists (only uncommented lines)
+        unless has_assets_bundler
+          # Track whether we successfully added the key
+          added = false
+
+          # Add assets_bundler after javascript_transpiler if it exists (excluding commented lines)
+          if (match = content.match(/^[ \t]*(?![ \t]*#)javascript_transpiler:.*$/))
+            indent = match[0][/^[ \t]*/]
+            content.sub!(/^([ \t]*(?![ \t]*#)javascript_transpiler:.*$)/, "\\1\n#{assets_bundler_entry(bundler, indent)}")
+            added = true
+          # Otherwise, add it after source_path if it exists (excluding commented lines)
+          elsif (match = content.match(/^[ \t]*(?![ \t]*#)source_path:.*$/))
+            indent = match[0][/^[ \t]*/]
+            content.sub!(/^([ \t]*(?![ \t]*#)source_path:.*$)/, "\\1\n#{assets_bundler_entry(bundler, indent)}")
+            added = true
+          # Add it after default: &default if it exists
+          elsif content.match?(/^default:[ \t]*&default[ \t]*$/)
+            # Use default 2-space indentation for this case
+            content.sub!(/^(default:[ \t]*&default[ \t]*)$/, "\\1\n#{assets_bundler_entry(bundler, '  ')}")
+            added = true
+          # Fallback: add after "default:" with proper indentation detection (handles blank lines)
+          elsif (match = content.match(/^default:\s*\n\s*([ \t]+)/m))
+            # Extract indentation from first indented line after "default:"
+            indent = match[1]
+            content.sub!(/^(default:\s*)$/, "\\1\n#{assets_bundler_entry(bundler, indent)}")
+            added = true
+          end
 
-        # Replace assets_bundler value (handles spaces, tabs, and various quote styles)
-        # Only matches uncommented lines
-        content.gsub!(/^([ \t]*assets_bundler:[ \t]*['"]?)(webpack|rspack)(['"]?)/, "\\1#{bundler}\\3")
+          unless added
+            puts "⚠️  Warning: Could not find appropriate location for assets_bundler in config"
+            puts "   Please add 'assets_bundler: #{bundler}' to the default section manually"
+          end
+        else
+          # Replace existing assets_bundler value (handles spaces, tabs, and various quote styles)
+          # Only matches uncommented lines
+          content.gsub!(/^([ \t]*)(?![ \t]*#)(assets_bundler:[ \t]*['"]?)(webpack|rspack)(['"]?)/, "\\1\\2#{bundler}\\4")
+          added = true
+        end
 
         # Update javascript_transpiler recommendation for rspack
         # Only update if not already set to swc and only on uncommented lines
-        if bundler == "rspack" && content !~ /^[ \t]*javascript_transpiler:[ \t]*['"]?swc['"]?/
-          content.gsub!(/^([ \t]*javascript_transpiler:[ \t]*['"]?)\w+(['"]?)/, "\\1swc\\2")
+        if bundler == "rspack" && content !~ /^[ \t]*(?![ \t]*#)javascript_transpiler:[ \t]*['"]?swc['"]?/
+          content.gsub!(/^([ \t]*(?![ \t]*#)javascript_transpiler:[ \t]*['"]?)(\w+)(['"]?)/, '\1swc\3')
         end
 
         File.write(config_path, content)
+        added
+      end
+
+      # Verify that the config was updated successfully
+      def verify_config_update(bundler)
+        config = load_yaml_config(config_path)
+        actual_bundler = config.dig("default", "assets_bundler")
+
+        if actual_bundler != bundler
+          raise "Config update verification failed: expected assets_bundler to be '#{bundler}', but got '#{actual_bundler}'"
+        end
+      rescue Psych::SyntaxError => e
+        raise "Config update generated invalid YAML: #{e.message}"
+      end
+
+      # Generate the assets_bundler YAML entry with proper indentation
+      # @param bundler [String] The bundler name ('webpack' or 'rspack')
+      # @param indent [String] The indentation string to use (e.g., '  ' or '\t')
+      # @return [String] The formatted YAML entry
+      def assets_bundler_entry(bundler, indent)
+        "\n#{indent}# Select assets bundler to use\n#{indent}# Available options: 'webpack' (default) or 'rspack'\n#{indent}assets_bundler: \"#{bundler}\""
       end
 
       def manage_dependencies(bundler, install_deps, switching: true, no_uninstall: false)

spec/shakapacker/bundler_switcher_spec.rb

@@ -137,6 +137,120 @@ def load_yaml_for_test(path)
       expect(config["default"]["javascript_transpiler"]).to eq("swc")
     end
 
+    it "adds assets_bundler key when missing from config" do
+      config_without_assets_bundler = <<~YAML
+        default: &default
+          source_path: app/javascript
+          javascript_transpiler: babel
+
+        development:
+          <<: *default
+
+        production:
+          <<: *default
+      YAML
+      File.write(config_path, config_without_assets_bundler)
+
+      switcher.switch_to("rspack")
+      config = load_yaml_for_test(config_path)
+      expect(config["default"]["assets_bundler"]).to eq("rspack")
+    end
+
+    it "does not add assets_bundler key when switching to webpack without the key (using default)" do
+      config_minimal = <<~YAML
+        default: &default
+          source_path: app/javascript
+
+        development:
+          <<: *default
+      YAML
+      File.write(config_path, config_minimal)
+
+      # When switching to webpack without the key, it should recognize we're already using webpack
+      # via the default and not add an unnecessary key
+      expect do
+        switcher.switch_to("webpack")
+      end.to output(/Already using webpack/).to_stdout
+
+      config = load_yaml_for_test(config_path)
+      # Should NOT have added the key since we're already on webpack via default
+      expect(config.dig("default", "assets_bundler")).to be_nil
+    end
+
+    it "adds assets_bundler key when switching to rspack even without javascript_transpiler" do
+      config_minimal = <<~YAML
+        default: &default
+          source_path: app/javascript
+
+        development:
+          <<: *default
+      YAML
+      File.write(config_path, config_minimal)
+
+      # When switching to rspack, the key should be added even without javascript_transpiler
+      switcher.switch_to("rspack")
+      config = load_yaml_for_test(config_path)
+      expect(config["default"]["assets_bundler"]).to eq("rspack")
+    end
+
+    it "adds assets_bundler key to real-world config structure" do
+      # Based on https://github.com/swrobel/meta-surf-forecast/blob/8f54a2ca0a798b8724430e137d56dfcc5fcbe775/config/shakapacker.yml
+      config_real_world = <<~YAML
+        # Note: You must restart bin/webpack-dev-server for changes to take effect
+
+        default: &default
+          ensure_consistent_versioning: true
+          source_path: app/javascript
+          source_entry_path: /
+          public_root_path: public
+          public_output_path: packs
+          cache_path: tmp/cache/webpacker
+          check_yarn_integrity: false
+          webpack_compile_output: false
+          javascript_transpiler: 'swc'
+
+          # If nested_entries is true, then we'll pick up subdirectories within the source_entry_path.
+          nested_entries: false
+
+        development:
+          <<: *default
+          compile: true
+
+        production:
+          <<: *default
+          compile: false
+      YAML
+      File.write(config_path, config_real_world)
+
+      switcher.switch_to("rspack")
+      config = load_yaml_for_test(config_path)
+      expect(config["default"]["assets_bundler"]).to eq("rspack")
+      # Verify it was added after javascript_transpiler with proper indentation
+      content = File.read(config_path)
+      expect(content).to match(/javascript_transpiler:.*\n\n.*assets_bundler:/m)
+    end
+
+    it "treats commented-out assets_bundler as missing" do
+      config_with_commented_key = <<~YAML
+        default: &default
+          source_path: app/javascript
+          javascript_transpiler: babel
+          # assets_bundler: webpack
+
+        development:
+          <<: *default
+      YAML
+      File.write(config_path, config_with_commented_key)
+
+      switcher.switch_to("rspack")
+      config = load_yaml_for_test(config_path)
+      # Should add a new uncommented key
+      expect(config["default"]["assets_bundler"]).to eq("rspack")
+      # Should preserve the commented line
+      content = File.read(config_path)
+      expect(content).to include("# assets_bundler: webpack")
+    end
+
     it "preserves config file structure and comments" do
       config_with_comments = <<~YAML
         # This is a comment
@@ -156,6 +270,115 @@ def load_yaml_for_test(path)
       expect(updated_content).to include("# inline comment")
     end
 
+    it "handles non-standard config with fallback pattern" do
+      # Config with no javascript_transpiler, source_path, or standard anchor
+      config_non_standard = <<~YAML
+        default: &default
+          public_root_path: public
+          public_output_path: packs
+          cache_path: tmp/cache/webpacker
+
+        development:
+          <<: *default
+      YAML
+      File.write(config_path, config_non_standard)
+
+      # Should use the fallback pattern to add after "default:" with detected indentation
+      switcher.switch_to("rspack")
+      config = load_yaml_for_test(config_path)
+      expect(config["default"]["assets_bundler"]).to eq("rspack")
+    end
+
+    it "warns when config structure is completely incompatible" do
+      # Completely unusual structure with no recognizable patterns
+      config_incompatible = <<~YAML
+        production_only: &production
+          cache_path: tmp/cache
+      YAML
+      File.write(config_path, config_incompatible)
+
+      expect do
+        switcher.switch_to("rspack")
+      end.to output(/Warning: Could not find appropriate location/).to_stdout
+
+      # Should not have added the key since structure was incompatible
+      config = load_yaml_for_test(config_path)
+      expect(config.dig("default", "assets_bundler")).to be_nil
+    end
+
+    it "does not match commented javascript_transpiler lines" do
+      config_with_commented = <<~YAML
+        default: &default
+          source_path: app/javascript
+          # javascript_transpiler: babel
+          other_key: value
+
+        development:
+          <<: *default
+      YAML
+      File.write(config_path, config_with_commented)
+
+      switcher.switch_to("rspack")
+      content = File.read(config_path)
+      # Should not add after the commented line
+      expect(content).not_to match(/# javascript_transpiler:.*\n.*assets_bundler:/)
+      # Should add after source_path instead (with single newline)
+      expect(content).to match(/source_path:.*\n\n.*# Select assets bundler/)
+    end
+
+    it "preserves inline comments when adding assets_bundler" do
+      config_with_inline = <<~YAML
+        default: &default
+          source_path: app/javascript  # Important path
+          javascript_transpiler: babel # Keep this
+
+        development:
+          <<: *default
+      YAML
+      File.write(config_path, config_with_inline)
+
+      switcher.switch_to("rspack")
+      content = File.read(config_path)
+      # Should preserve inline comments
+      expect(content).to include("source_path: app/javascript  # Important path")
+      expect(content).to include("javascript_transpiler: swc # Keep this")
+    end
+
+    it "handles config with blank lines after default" do
+      config_with_blanks = <<~YAML
+        default: &default
+
+          source_path: app/javascript
+          javascript_transpiler: babel
+
+        development:
+          <<: *default
+      YAML
+      File.write(config_path, config_with_blanks)
+
+      switcher.switch_to("rspack")
+      config = load_yaml_for_test(config_path)
+      expect(config["default"]["assets_bundler"]).to eq("rspack")
+    end
+
+    it "handles config with multiple blank lines after default" do
+      config_many_blanks = <<~YAML
+        default: &default
+
+
+          source_path: app/javascript
+          javascript_transpiler: babel
+
+        development:
+          <<: *default
+      YAML
+      File.write(config_path, config_many_blanks)
+
+      switcher.switch_to("rspack")
+      config = load_yaml_for_test(config_path)
+      expect(config["default"]["assets_bundler"]).to eq("rspack")
+    end
+
     context "when already using the target bundler" do
       it "does not reinstall deps when install_deps is false" do
         expect(switcher).not_to receive(:system)