Skip generate_packs when shakapacker precompile hook configured (#1861)

Skip generate_packs when shakapacker precompile hook configured (#1861)

Why
Prevent duplicate pack generation when shakapacker handles it via hooks.

Summary
Detect shakapacker precompile hooks and skip react_on_rails:generate_packs
accordingly. Support both direct commands and script file detection.

Key improvements
- Auto-detect hooks in shakapacker.yml (symbol/string keys, script files)
- Display hook value in logs for transparency
- Fail-safe approach ensures packs generate if detection fails

Impact
- Existing: No change unless shakapacker hook is configured
- New: Automatic skip when hook contains react_on_rails:generate_packs

Risks
None. Detection failures default to running generate_packs (safe).

Author: justin808

lib/react_on_rails/configuration.rb

@@ -238,7 +238,14 @@ def adjust_precompile_task
       raise(ReactOnRails::Error, compile_command_conflict_message) if ReactOnRails::PackerUtils.precompile?
 
       precompile_tasks = lambda {
-        Rake::Task["react_on_rails:generate_packs"].invoke
+        # Skip generate_packs if shakapacker has a precompile hook configured
+        if ReactOnRails::PackerUtils.shakapacker_precompile_hook_configured?
+          hook_value = ReactOnRails::PackerUtils.shakapacker_precompile_hook_value
+          puts "Skipping react_on_rails:generate_packs (configured in shakapacker precompile hook: #{hook_value})"
+        else
+          Rake::Task["react_on_rails:generate_packs"].invoke
+        end
+
         Rake::Task["react_on_rails:assets:webpack"].invoke
 
         # VERSIONS is per the shakacode/shakapacker clean method definition.

lib/react_on_rails/dev/pack_generator.rb

@@ -5,9 +5,36 @@
 
 module ReactOnRails
   module Dev
+    # PackGenerator triggers the generation of React on Rails packs
+    #
+    # Design decisions:
+    # 1. Why trigger via Rake task instead of direct Ruby code?
+    #    - The actual pack generation logic lives in lib/react_on_rails/packs_generator.rb
+    #    - The rake task (lib/tasks/generate_packs.rake) provides a stable, documented interface
+    #    - This allows the implementation to evolve without breaking bin/dev
+    #    - Users can also run the task directly: `rake react_on_rails:generate_packs`
+    #
+    # 2. Why two execution strategies (direct vs bundle exec)?
+    #    - Direct Rake execution: Faster when already in Bundler/Rails context (bin/dev)
+    #    - Bundle exec fallback: Required when called outside Rails context
+    #    - This optimization avoids subprocess overhead in the common case
+    #
+    # 3. Why is the class named "PackGenerator" when it delegates?
+    #    - It's a semantic wrapper around pack generation for the dev workflow
+    #    - Provides a clean API for bin/dev without exposing Rake internals
+    #    - Handles hook detection, error handling, and output formatting
     class PackGenerator
       class << self
         def generate(verbose: false)
+          # Skip if shakapacker has a precompile hook configured
+          if ReactOnRails::PackerUtils.shakapacker_precompile_hook_configured?
+            if verbose
+              hook_value = ReactOnRails::PackerUtils.shakapacker_precompile_hook_value
+              puts "⏭️  Skipping pack generation (handled by shakapacker precompile hook: #{hook_value})"
+            end
+            return
+          end
+
           if verbose
             puts "📦 Generating React on Rails packs..."
             success = run_pack_generation

lib/react_on_rails/packer_utils.rb

@@ -166,5 +166,78 @@ def self.raise_shakapacker_version_incompatible_for_basic_pack_generation
 
       raise ReactOnRails::Error, msg
     end
+
+    # Check if shakapacker.yml has a precompile hook configured
+    # This prevents react_on_rails from running generate_packs twice
+    #
+    # Returns false if detection fails for any reason (missing shakapacker, malformed config, etc.)
+    # to ensure generate_packs runs rather than being incorrectly skipped
+    #
+    # Note: Currently checks a single hook value. Future enhancement will support hook lists
+    # to allow prepending/appending multiple commands. See related Shakapacker issue for details.
+    def self.shakapacker_precompile_hook_configured?
+      return false unless defined?(::Shakapacker)
+
+      hook_value = extract_precompile_hook
+      return false if hook_value.nil?
+
+      hook_contains_generate_packs?(hook_value)
+    rescue StandardError => e
+      # Swallow errors during hook detection to fail safe - if we can't detect the hook,
+      # we should run generate_packs rather than skip it incorrectly.
+      # Possible errors: NoMethodError (config method missing), TypeError (unexpected data structure),
+      # or errors from shakapacker's internal implementation changes
+      warn "Warning: Unable to detect shakapacker precompile hook: #{e.message}" if ENV["DEBUG"]
+      false
+    end
+
+    def self.extract_precompile_hook
+      # Access config data using private :data method since there's no public API
+      # to access the raw configuration hash needed for hook detection
+      config_data = ::Shakapacker.config.send(:data)
+
+      # Try symbol keys first (Shakapacker's internal format), then fall back to string keys
+      # Note: Currently only one hook value is supported, but this will change to support lists
+      config_data&.dig(:hooks, :precompile) || config_data&.dig("hooks", "precompile")
+    end
+
+    def self.hook_contains_generate_packs?(hook_value)
+      # The hook value can be either:
+      # 1. A direct command containing the rake task
+      # 2. A path to a script file that needs to be read
+      return false if hook_value.blank?
+
+      # Check if it's a direct command first
+      return true if hook_value.to_s.match?(/\breact_on_rails:generate_packs\b/)
+
+      # Check if it's a script file path
+      script_path = resolve_hook_script_path(hook_value)
+      return false unless script_path && File.exist?(script_path)
+
+      # Read and check script contents
+      script_contents = File.read(script_path)
+      script_contents.match?(/\breact_on_rails:generate_packs\b/)
+    rescue StandardError
+      # If we can't read the script, assume it doesn't contain generate_packs
+      false
+    end
+
+    def self.resolve_hook_script_path(hook_value)
+      # Hook value might be a script path relative to Rails root
+      return nil unless defined?(Rails) && Rails.respond_to?(:root)
+
+      potential_path = Rails.root.join(hook_value.to_s.strip)
+      potential_path if potential_path.file?
+    end
+
+    # Returns the configured precompile hook value for logging/debugging
+    # Returns nil if no hook is configured
+    def self.shakapacker_precompile_hook_value
+      return nil unless defined?(::Shakapacker)
+
+      extract_precompile_hook
+    rescue StandardError
+      nil
+    end
   end
 end