Address version requirement review comments

- Add MINIMUM_SHAKAPACKER_VERSION_FOR_AUTO_REGISTRATION constant to clarify different version requirements
- Update supports_auto_registration? to use new constant
- Add supports_basic_pack_generation? method for backward compatibility
- Update configuration checks to allow basic pack generation for 6.5.1+ while requiring 7.0+ for advanced features
- Improve error messages to be more specific about version requirements
- Update system checker warning to clarify nested entries support requirement
- Update tests to use instance_double for better type safety
- Add tests for new supports_basic_pack_generation? method

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

Co-Authored-By: Claude <[email protected]>

Author: justin808

lib/react_on_rails/configuration.rb

@@ -198,9 +198,19 @@ def check_autobundling_requirements
       raise_missing_components_subdirectory if auto_load_bundle && !components_subdirectory.present?
       return unless components_subdirectory.present?
 
-      ReactOnRails::PackerUtils.raise_shakapacker_version_incompatible_for_autobundling unless
-        ReactOnRails::PackerUtils.supports_auto_registration?
-      ReactOnRails::PackerUtils.raise_nested_entries_disabled unless ReactOnRails::PackerUtils.nested_entries?
+      # Check basic pack generation support for auto_load_bundle
+      ReactOnRails::PackerUtils.raise_shakapacker_version_incompatible_for_basic_pack_generation unless
+        ReactOnRails::PackerUtils.supports_basic_pack_generation?
+
+      # Additional checks for advanced features requiring nested entries
+      if ReactOnRails::PackerUtils.supports_auto_registration?
+        ReactOnRails::PackerUtils.raise_nested_entries_disabled unless ReactOnRails::PackerUtils.nested_entries?
+      else
+        # Warn users about missing advanced features but don't block basic functionality
+        min_version = ReactOnRails::PacksGenerator::MINIMUM_SHAKAPACKER_VERSION_FOR_AUTO_REGISTRATION
+        Rails.logger.warn("React on Rails: Basic pack generation enabled. " \
+                          "Upgrade to Shakapacker #{min_version}+ for advanced auto-registration features.")
+      end
     end
 
     def adjust_precompile_task

lib/react_on_rails/packer_utils.rb

@@ -38,9 +38,13 @@ def self.supports_async_loading?
       shakapacker_version_requirement_met?("8.2.0")
     end
 
+    def self.supports_basic_pack_generation?
+      shakapacker_version_requirement_met?(ReactOnRails::PacksGenerator::MINIMUM_SHAKAPACKER_VERSION)
+    end
+
     def self.supports_auto_registration?
-      packer.config.respond_to?(:nested_entries?) &&
-        shakapacker_version_requirement_met?("7.0.0")
+      min_version = ReactOnRails::PacksGenerator::MINIMUM_SHAKAPACKER_VERSION_FOR_AUTO_REGISTRATION
+      packer.config.respond_to?(:nested_entries?) && shakapacker_version_requirement_met?(min_version)
     end
 
     # This returns either a URL for the webpack-dev-server, non-server bundle or
@@ -142,10 +146,20 @@ def self.raise_nested_entries_disabled
     end
 
     def self.raise_shakapacker_version_incompatible_for_autobundling
+      msg = <<~MSG
+        **ERROR** ReactOnRails: Please upgrade Shakapacker to version #{ReactOnRails::PacksGenerator::MINIMUM_SHAKAPACKER_VERSION_FOR_AUTO_REGISTRATION} or \
+        above to use the automated bundle generation feature (which requires nested_entries support). \
+        The currently installed version is #{ReactOnRails::PackerUtils.shakapacker_version}. \
+        Basic pack generation requires Shakapacker #{ReactOnRails::PacksGenerator::MINIMUM_SHAKAPACKER_VERSION} or above.
+      MSG
+
+      raise ReactOnRails::Error, msg
+    end
+
+    def self.raise_shakapacker_version_incompatible_for_basic_pack_generation
       msg = <<~MSG
         **ERROR** ReactOnRails: Please upgrade Shakapacker to version #{ReactOnRails::PacksGenerator::MINIMUM_SHAKAPACKER_VERSION} or \
-        above to use the automated bundle generation feature. The currently installed version is \
-        #{ReactOnRails::PackerUtils.shakapacker_version}.
+        above to use basic pack generation features. The currently installed version is #{ReactOnRails::PackerUtils.shakapacker_version}.
       MSG
 
       raise ReactOnRails::Error, msg

lib/react_on_rails/packs_generator.rb

@@ -9,6 +9,8 @@ class PacksGenerator
     CONTAINS_CLIENT_OR_SERVER_REGEX = /\.(server|client)($|\.)/
     COMPONENT_EXTENSIONS = /\.(jsx?|tsx?)$/
     MINIMUM_SHAKAPACKER_VERSION = "6.5.1"
+    # Auto-registration requires nested_entries support which was added in 7.0.0
+    MINIMUM_SHAKAPACKER_VERSION_FOR_AUTO_REGISTRATION = "7.0.0"
 
     def self.instance
       @instance ||= PacksGenerator.new

lib/react_on_rails/system_checker.rb

@@ -624,7 +624,7 @@ def report_shakapacker_version_with_threshold
             if ReactOnRails::PackerUtils.supports_auto_registration?
               add_success("✅ Shakapacker #{version} (supports React on Rails auto-registration)")
             else
-              add_warning("⚠️  Shakapacker #{version} - Version 7.0+ needed for React on Rails auto-registration")
+              add_warning("⚠️  Shakapacker #{version} - Version 7.0+ needed for auto-registration (nested entries)")
             end
           rescue ArgumentError
             # Fallback for invalid version strings

spec/lib/react_on_rails/doctor_spec.rb

@@ -1,7 +1,5 @@
 # frozen_string_literal: true
 
-# rubocop:disable RSpec/VerifiedDoubles
-
 require_relative "../../react_on_rails/spec_helper"
 require_relative "../../../lib/react_on_rails/doctor"
 
@@ -89,13 +87,15 @@
 
     describe "#determine_server_bundle_path" do
       context "when Shakapacker gem is available with relative paths" do
-        let(:shakapacker_config) { double(source_path: "client/app", source_entry_path: "packs") }
+        let(:shakapacker_config) do
+          instance_double(Shakapacker::Configuration, source_path: "client/app", source_entry_path: "packs")
+        end
 
         before do
-          shakapacker_module = double("Shakapacker", config: shakapacker_config)
+          shakapacker_module = instance_double(Shakapacker, config: shakapacker_config)
           stub_const("Shakapacker", shakapacker_module)
           allow(doctor).to receive(:require).with("shakapacker").and_return(true)
-          allow(doctor).to receive(:get_server_bundle_filename).and_return("server-bundle.js")
+          allow(doctor).to receive(:server_bundle_filename).and_return("server-bundle.js")
         end
 
         it "uses Shakapacker API configuration with relative paths" do
@@ -106,13 +106,16 @@
 
       context "when Shakapacker gem is available with absolute paths" do
         let(:rails_root) { "/Users/test/myapp" }
-        let(:shakapacker_config) { double(source_path: "#{rails_root}/client/app", source_entry_path: "packs") }
+        let(:shakapacker_config) do
+          instance_double(Shakapacker::Configuration, source_path: "#{rails_root}/client/app",
+                                                      source_entry_path: "packs")
+        end
 
         before do
-          shakapacker_module = double("Shakapacker", config: shakapacker_config)
+          shakapacker_module = instance_double(Shakapacker, config: shakapacker_config)
           stub_const("Shakapacker", shakapacker_module)
           allow(doctor).to receive(:require).with("shakapacker").and_return(true)
-          allow(doctor).to receive(:get_server_bundle_filename).and_return("server-bundle.js")
+          allow(doctor).to receive(:server_bundle_filename).and_return("server-bundle.js")
           allow(Dir).to receive(:pwd).and_return(rails_root)
         end
 
@@ -125,14 +128,15 @@
       context "when Shakapacker gem returns nested absolute paths" do
         let(:rails_root) { "/Users/test/myapp" }
         let(:shakapacker_config) do
-          double(source_path: "#{rails_root}/client/app", source_entry_path: "#{rails_root}/client/app/packs")
+          instance_double(Shakapacker::Configuration, source_path: "#{rails_root}/client/app",
+                                                      source_entry_path: "#{rails_root}/client/app/packs")
         end
 
         before do
-          shakapacker_module = double("Shakapacker", config: shakapacker_config)
+          shakapacker_module = instance_double(Shakapacker, config: shakapacker_config)
           stub_const("Shakapacker", shakapacker_module)
           allow(doctor).to receive(:require).with("shakapacker").and_return(true)
-          allow(doctor).to receive(:get_server_bundle_filename).and_return("server-bundle.js")
+          allow(doctor).to receive(:server_bundle_filename).and_return("server-bundle.js")
           allow(Dir).to receive(:pwd).and_return(rails_root)
         end
 
@@ -145,7 +149,7 @@
       context "when Shakapacker gem is not available" do
         before do
           allow(doctor).to receive(:require).with("shakapacker").and_raise(LoadError)
-          allow(doctor).to receive(:get_server_bundle_filename).and_return("server-bundle.js")
+          allow(doctor).to receive(:server_bundle_filename).and_return("server-bundle.js")
         end
 
         it "uses default path" do
@@ -155,7 +159,7 @@
       end
     end
 
-    describe "#get_server_bundle_filename" do
+    describe "#server_bundle_filename" do
       context "when react_on_rails.rb has custom filename" do
         let(:initializer_content) do
           'config.server_bundle_js_file = "custom-server-bundle.js"'
@@ -167,7 +171,7 @@
         end
 
         it "extracts filename from initializer" do
-          filename = doctor.send(:get_server_bundle_filename)
+          filename = doctor.send(:server_bundle_filename)
           expect(filename).to eq("custom-server-bundle.js")
         end
       end
@@ -178,12 +182,10 @@
         end
 
         it "returns default filename" do
-          filename = doctor.send(:get_server_bundle_filename)
+          filename = doctor.send(:server_bundle_filename)
           expect(filename).to eq("server-bundle.js")
         end
       end
     end
   end
 end
-
-# rubocop:enable RSpec/VerifiedDoubles

spec/react_on_rails/packer_utils_spec.rb

@@ -91,31 +91,50 @@ module ReactOnRails
       end
     end
 
+    describe ".supports_basic_pack_generation?" do
+      it "returns true when Shakapacker >= 6.5.1" do
+        allow(described_class).to receive(:shakapacker_version_requirement_met?)
+          .with(ReactOnRails::PacksGenerator::MINIMUM_SHAKAPACKER_VERSION).and_return(true)
+
+        expect(described_class.supports_basic_pack_generation?).to be(true)
+      end
+
+      it "returns false when Shakapacker < 6.5.1" do
+        allow(described_class).to receive(:shakapacker_version_requirement_met?)
+          .with(ReactOnRails::PacksGenerator::MINIMUM_SHAKAPACKER_VERSION).and_return(false)
+
+        expect(described_class.supports_basic_pack_generation?).to be(false)
+      end
+    end
+
     describe ".supports_auto_registration?" do
-      let(:mock_config) { double("MockConfig") } # rubocop:disable RSpec/VerifiedDoubles
-      let(:mock_packer) { double("MockPacker", config: mock_config) } # rubocop:disable RSpec/VerifiedDoubles
+      let(:mock_config) { instance_double(Shakapacker::Config) }
+      let(:mock_packer) { instance_double(Shakapacker, config: mock_config) }
 
       before do
         allow(described_class).to receive(:packer).and_return(mock_packer)
       end
 
       it "returns true when Shakapacker >= 7.0.0 with nested_entries support" do
         allow(mock_config).to receive(:respond_to?).with(:nested_entries?).and_return(true)
-        allow(described_class).to receive(:shakapacker_version_requirement_met?).with("7.0.0").and_return(true)
+        allow(described_class).to receive(:shakapacker_version_requirement_met?)
+          .with(ReactOnRails::PacksGenerator::MINIMUM_SHAKAPACKER_VERSION_FOR_AUTO_REGISTRATION).and_return(true)
 
         expect(described_class.supports_auto_registration?).to be(true)
       end
 
       it "returns false when Shakapacker < 7.0.0" do
         allow(mock_config).to receive(:respond_to?).with(:nested_entries?).and_return(true)
-        allow(described_class).to receive(:shakapacker_version_requirement_met?).with("7.0.0").and_return(false)
+        allow(described_class).to receive(:shakapacker_version_requirement_met?)
+          .with(ReactOnRails::PacksGenerator::MINIMUM_SHAKAPACKER_VERSION_FOR_AUTO_REGISTRATION).and_return(false)
 
         expect(described_class.supports_auto_registration?).to be(false)
       end
 
       it "returns false when nested_entries method is not available" do
         allow(mock_config).to receive(:respond_to?).with(:nested_entries?).and_return(false)
-        allow(described_class).to receive(:shakapacker_version_requirement_met?).with("7.0.0").and_return(true)
+        allow(described_class).to receive(:shakapacker_version_requirement_met?)
+          .with(ReactOnRails::PacksGenerator::MINIMUM_SHAKAPACKER_VERSION_FOR_AUTO_REGISTRATION).and_return(true)
 
         expect(described_class.supports_auto_registration?).to be(false)
       end