Fix CI failure by skipping version validation during generator runtime (#1918)

* Fix CI failure by skipping version validation during generator runtime

The strict version validation added in #1881 runs during Rails initialization
via an after_initialize hook. However, when running `rails generate react_on_rails:install`,
the npm packages haven't been installed yet, causing the validation to fail with:
"No React on Rails npm package is installed."

This fix adds a check to skip validation when running Rails generators (detected by
checking if ARGV.first is "generate" or "g"). The generator will install packages
during its execution, so validation at initialization time is not appropriate.

This allows CI to successfully run example generation tasks without errors.

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

Co-Authored-By: Claude <noreply@anthropic.com>

* Address code review feedback: improve validation skip logic

Improvements based on code review:

1. **Nil-safe ARGV check**: Use safe navigation operator and check for empty ARGV
   - Prevents errors when ARGV is empty or nil
   - More defensive against edge cases

2. **Refactored to extract methods**: Created testable class methods
   - skip_version_validation?() - main entry point
   - running_generator?() - checks if running Rails generator
   - package_json_missing?() - checks if package.json exists
   - Improves testability and code clarity

3. **Added debug logging**: Log when validation is skipped
   - "Skipping validation - package.json not found"
   - "Skipping validation during generator runtime"
   - Helps debugging and understanding behavior

4. **Optimized check order**: Check package.json existence first
   - File existence check is cheaper than ARGV inspection
   - Handles more cases (initial setup, generators, etc.)

5. **Comprehensive test coverage**: Added 15 test cases
   - Tests for package.json missing scenarios
   - Tests for generator detection (both "generate" and "g")
   - Tests for edge cases (empty ARGV, other commands)
   - Tests for each helper method

All tests passing, RuboCop clean.

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

Co-Authored-By: Claude <noreply@anthropic.com>

* Fix NoMethodError: call Engine.skip_version_validation? as class method

The initializer block context doesn't have direct access to class methods.
Changed from `skip_version_validation?` to `Engine.skip_version_validation?`
to properly call the class method.

Fixes error:
NoMethodError: undefined method `skip_version_validation?' for #<ReactOnRails::Engine>

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

Co-Authored-By: Claude <noreply@anthropic.com>

---------

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

Author: justin808

lib/react_on_rails/engine.rb

@@ -6,19 +6,47 @@ module ReactOnRails
   class Engine < ::Rails::Engine
     # Validate package versions and compatibility on Rails startup
     # This ensures the application fails fast if versions don't match or packages are misconfigured
-    # Skip validation during installation tasks (e.g., shakapacker:install)
+    # Skip validation during installation tasks (e.g., shakapacker:install) or generator runtime
     initializer "react_on_rails.validate_version_and_package_compatibility" do
       config.after_initialize do
-        # Skip validation if package.json doesn't exist yet (during initial setup)
-        package_json = VersionChecker::NodePackageVersion.package_json_path
-        next unless File.exist?(package_json)
+        next if Engine.skip_version_validation?
 
         Rails.logger.info "[React on Rails] Validating package version and compatibility..."
         VersionChecker.build.validate_version_and_package_compatibility!
         Rails.logger.info "[React on Rails] Package validation successful"
       end
     end
 
+    # Determine if version validation should be skipped
+    # @return [Boolean] true if validation should be skipped
+    def self.skip_version_validation?
+      # Check package.json first as it's cheaper and handles more cases
+      if package_json_missing?
+        Rails.logger.debug "[React on Rails] Skipping validation - package.json not found"
+        return true
+      end
+
+      # Skip during generator runtime since packages are installed during execution
+      if running_generator?
+        Rails.logger.debug "[React on Rails] Skipping validation during generator runtime"
+        return true
+      end
+
+      false
+    end
+
+    # Check if we're running a Rails generator
+    # @return [Boolean] true if running a generator
+    def self.running_generator?
+      !ARGV.empty? && ARGV.first&.in?(%w[generate g])
+    end
+
+    # Check if package.json doesn't exist yet
+    # @return [Boolean] true if package.json is missing
+    def self.package_json_missing?
+      !File.exist?(VersionChecker::NodePackageVersion.package_json_path)
+    end
+
     config.to_prepare do
       ReactOnRails::ServerRenderingPool.reset_pool
     end

spec/react_on_rails/engine_spec.rb

@@ -0,0 +1,160 @@
+# frozen_string_literal: true
+
+require_relative "spec_helper"
+
+module ReactOnRails
+  RSpec.describe Engine do
+    describe ".skip_version_validation?" do
+      let(:package_json_path) { "/fake/path/package.json" }
+
+      before do
+        allow(VersionChecker::NodePackageVersion).to receive(:package_json_path)
+          .and_return(package_json_path)
+        allow(Rails.logger).to receive(:debug)
+      end
+
+      context "when package.json doesn't exist" do
+        before do
+          allow(File).to receive(:exist?).with(package_json_path).and_return(false)
+        end
+
+        it "returns true" do
+          expect(described_class.skip_version_validation?).to be true
+        end
+
+        it "logs debug message about missing package.json" do
+          described_class.skip_version_validation?
+          expect(Rails.logger).to have_received(:debug)
+            .with("[React on Rails] Skipping validation - package.json not found")
+        end
+      end
+
+      context "when package.json exists" do
+        before do
+          allow(File).to receive(:exist?).with(package_json_path).and_return(true)
+        end
+
+        context "when running a generator" do
+          before do
+            stub_const("ARGV", ["generate", "react_on_rails:install"])
+          end
+
+          it "returns true" do
+            expect(described_class.skip_version_validation?).to be true
+          end
+
+          it "logs debug message about generator runtime" do
+            described_class.skip_version_validation?
+            expect(Rails.logger).to have_received(:debug)
+              .with("[React on Rails] Skipping validation during generator runtime")
+          end
+        end
+
+        context "when running a generator with short form" do
+          before do
+            stub_const("ARGV", ["g", "react_on_rails:install"])
+          end
+
+          it "returns true" do
+            expect(described_class.skip_version_validation?).to be true
+          end
+        end
+
+        context "when ARGV is empty" do
+          before do
+            stub_const("ARGV", [])
+          end
+
+          it "returns false" do
+            expect(described_class.skip_version_validation?).to be false
+          end
+        end
+
+        context "when running other commands" do
+          %w[server console runner].each do |command|
+            context "when running '#{command}'" do
+              before do
+                stub_const("ARGV", [command])
+              end
+
+              it "returns false" do
+                expect(described_class.skip_version_validation?).to be false
+              end
+            end
+          end
+        end
+      end
+    end
+
+    describe ".running_generator?" do
+      context "when ARGV is empty" do
+        before do
+          stub_const("ARGV", [])
+        end
+
+        it "returns false" do
+          expect(described_class.running_generator?).to be false
+        end
+      end
+
+      context "when ARGV.first is 'generate'" do
+        before do
+          stub_const("ARGV", %w[generate model User])
+        end
+
+        it "returns true" do
+          expect(described_class.running_generator?).to be true
+        end
+      end
+
+      context "when ARGV.first is 'g'" do
+        before do
+          stub_const("ARGV", %w[g controller Users])
+        end
+
+        it "returns true" do
+          expect(described_class.running_generator?).to be true
+        end
+      end
+
+      context "when ARGV.first is another command" do
+        before do
+          stub_const("ARGV", ["server"])
+        end
+
+        it "returns false" do
+          expect(described_class.running_generator?).to be false
+        end
+      end
+    end
+
+    describe ".package_json_missing?" do
+      let(:package_json_path) { "/fake/path/package.json" }
+
+      before do
+        allow(VersionChecker::NodePackageVersion).to receive(:package_json_path)
+          .and_return(package_json_path)
+      end
+
+      context "when package.json exists" do
+        before do
+          allow(File).to receive(:exist?).with(package_json_path).and_return(true)
+        end
+
+        it "returns false" do
+          expect(described_class.package_json_missing?).to be false
+        end
+      end
+
+      context "when package.json doesn't exist" do
+        before do
+          allow(File).to receive(:exist?).with(package_json_path).and_return(false)
+        end
+
+        it "returns true" do
+          expect(described_class.package_json_missing?).to be true
+        end
+      end
+    end
+  end
+end