refactor: Simplify JS dependency manager by removing redundant package_json checks

Since react_on_rails requires shakapacker (>= 6.0) and shakapacker includes
package_json as a dependency, the package_json gem is always available.

Changes:
- Remove package_json_available? method and all fallback logic
- Simplify version detection to always use exact gem version
- Remove detect_and_run_package_manager_install (package_json handles this)
- Remove @ran_direct_installs tracking (no longer needed)
- Update tests to remove fallback scenarios
- Update documentation to reflect guaranteed package_json availability

This eliminates ~60 lines of redundant code and makes the module more
maintainable and less error-prone.

Co-authored-by: Justin Gordon <justin808@users.noreply.github.com>

Author: github-actions[bot]

lib/generators/react_on_rails/js_dependency_manager.rb

@@ -8,21 +8,22 @@ module Generators
     # This module provides common functionality for adding and installing
     # JS dependencies to avoid code duplication between generators.
     #
+    # Since react_on_rails requires shakapacker, and shakapacker includes
+    # package_json as a dependency, the package_json gem is always available.
+    #
     # == Required Instance Variables
     # Including classes must support these instance variables:
     # - @added_dependencies_to_package_json: Boolean tracking if package_json gem was used
-    # - @ran_direct_installs: Boolean tracking if direct npm/yarn commands were run
     #
     # == Required Methods
     # Including classes must include GeneratorHelper module which provides:
     # - add_npm_dependencies(packages, dev: false): Add packages via package_json gem
-    # - package_json: Access to PackageJson instance
+    # - package_json: Access to PackageJson instance (always available via shakapacker)
     # - destination_root: Generator destination directory
-    # - system(*args): Execute system commands
     #
     # == Usage
     # Include this module in generator classes and call setup_js_dependencies
-    # to handle all JS dependency installation with automatic fallbacks.
+    # to handle all JS dependency installation via package_json gem.
     module JsDependencyManager
       # Core React dependencies required for React on Rails
       REACT_DEPENDENCIES = %w[
@@ -52,7 +53,6 @@ module JsDependencyManager
 
       def setup_js_dependencies
         @added_dependencies_to_package_json ||= false
-        @ran_direct_installs ||= false
         add_js_dependencies
         install_js_dependencies
       end
@@ -65,25 +65,15 @@ def add_js_dependencies
       end
 
       def add_react_on_rails_package
-        major_minor_patch_only = /\A\d+\.\d+\.\d+\z/
-
-        react_on_rails_pkg = if ReactOnRails::VERSION.match?(major_minor_patch_only)
-                               "react-on-rails@#{ReactOnRails::VERSION}"
-                             else
-                               puts "Adding the latest react-on-rails NPM module. " \
-                                    "Double check this is correct in package.json"
-                               "react-on-rails"
-                             end
+        # Always use exact version match between gem and npm package
+        react_on_rails_pkg = "react-on-rails@#{ReactOnRails::VERSION}"
 
         puts "Installing React on Rails package..."
         if add_js_dependency(react_on_rails_pkg)
           @added_dependencies_to_package_json = true
         else
-          # Fallback to direct npm install
-          puts "Using direct npm commands as fallback"
-          success = system("npm", "install", react_on_rails_pkg)
-          @ran_direct_installs = true if success
-          handle_npm_failure("react-on-rails package", [react_on_rails_pkg]) unless success
+          # This should not happen since package_json is always available via shakapacker
+          raise "Failed to add react-on-rails package via package_json gem"
         end
       end
 
@@ -93,10 +83,8 @@ def add_react_dependencies
         if add_js_dependencies_batch(REACT_DEPENDENCIES)
           @added_dependencies_to_package_json = true
         else
-          # Fallback to direct npm install
-          success = system("npm", "install", *REACT_DEPENDENCIES)
-          @ran_direct_installs = true if success
-          handle_npm_failure("React dependencies", REACT_DEPENDENCIES) unless success
+          # This should not happen since package_json is always available via shakapacker
+          raise "Failed to add React dependencies via package_json gem"
         end
       end
 
@@ -106,10 +94,8 @@ def add_css_dependencies
         if add_js_dependencies_batch(CSS_DEPENDENCIES)
           @added_dependencies_to_package_json = true
         else
-          # Fallback to direct npm install
-          success = system("npm", "install", *CSS_DEPENDENCIES)
-          @ran_direct_installs = true if success
-          handle_npm_failure("CSS dependencies", CSS_DEPENDENCIES) unless success
+          # This should not happen since package_json is always available via shakapacker
+          raise "Failed to add CSS dependencies via package_json gem"
         end
       end
 
@@ -119,17 +105,13 @@ def add_dev_dependencies
         if add_js_dependencies_batch(DEV_DEPENDENCIES, dev: true)
           @added_dependencies_to_package_json = true
         else
-          # Fallback to direct npm install
-          success = system("npm", "install", "--save-dev", *DEV_DEPENDENCIES)
-          @ran_direct_installs = true if success
-          handle_npm_failure("development dependencies", DEV_DEPENDENCIES, dev: true) unless success
+          # This should not happen since package_json is always available via shakapacker
+          raise "Failed to add development dependencies via package_json gem"
         end
       end
 
       # Add a single dependency using package_json gem
       def add_js_dependency(package, dev: false)
-        return false unless package_json_available?
-
         pj = package_json
         return false unless pj
 
@@ -150,83 +132,30 @@ def add_js_dependency(package, dev: false)
 
       # Add multiple dependencies at once using package_json gem
       def add_js_dependencies_batch(packages, dev: false)
-        return false unless package_json_available?
-
         # Use the add_npm_dependencies helper from GeneratorHelper
         add_npm_dependencies(packages, dev: dev)
       end
 
-      # Check if package_json gem is available and loaded
-      def package_json_available?
-        # Check if Shakapacker or package_json gem is available
-        return true if defined?(PackageJson)
-
+      def install_js_dependencies
+        # Use package_json gem's install method (always available via shakapacker)
         begin
-          require "package_json"
+          package_json.manager.install
           true
-        rescue LoadError
-          false
-        end
-      end
-
-      def install_js_dependencies
-        # First try to use package_json gem's install method if available
-        if package_json_available? && package_json
-          begin
-            package_json.manager.install
-            return true
-          rescue StandardError => e
-            puts "Warning: package_json gem install failed: #{e.message}"
-            # Fall through to manual detection
-          end
-        end
-
-        # Fallback to detecting package manager and running install
-        success = detect_and_run_package_manager_install
-
-        unless success
+        rescue StandardError => e
           GeneratorMessages.add_warning(<<~MSG.strip)
-            ⚠️  JavaScript dependencies installation failed.
+            ⚠️  JavaScript dependencies installation failed: #{e.message}
 
-            This could be due to network issues or missing package manager.
+            This could be due to network issues or package manager problems.
             You can install dependencies manually later by running:
             • npm install (if using npm)
             • yarn install (if using yarn)
             • pnpm install (if using pnpm)
           MSG
-        end
-
-        success
-      end
-
-      def detect_and_run_package_manager_install
-        # Detect which package manager to use based on lock files
-        if File.exist?(File.join(destination_root, "yarn.lock"))
-          system("yarn", "install")
-        elsif File.exist?(File.join(destination_root, "pnpm-lock.yaml"))
-          system("pnpm", "install")
-        elsif File.exist?(File.join(destination_root, "package-lock.json")) ||
-              File.exist?(File.join(destination_root, "package.json"))
-          # Use npm for package-lock.json or as default fallback
-          system("npm", "install")
-        else
-          true # No package manager detected, skip
+          false
         end
       end
 
-      def handle_npm_failure(dependency_type, packages, dev: false)
-        install_command = dev ? "npm install --save-dev" : "npm install"
-        GeneratorMessages.add_warning(<<~MSG.strip)
-          ⚠️  Failed to install #{dependency_type}.
-
-          The following packages could not be installed automatically:
-          #{packages.map { |pkg| "  • #{pkg}" }.join("\n")}
-
-          This could be due to network issues or missing package manager.
-          You can install them manually later by running:
-            #{install_command} #{packages.join(' ')}
-        MSG
-      end
+      # No longer needed since package_json gem handles package manager detection
     end
   end
 end

spec/react_on_rails/generators/message_deduplication_spec.rb

@@ -79,12 +79,11 @@
 
       # Initialize instance variables
       install_generator.instance_variable_set(:@added_dependencies_to_package_json, false)
-      install_generator.instance_variable_set(:@ran_direct_installs, false)
     end
 
-    context "when using package_json gem" do
+    context "when using package_json gem (always available via shakapacker)" do
       before do
-        # Simply mock that the individual package_json gem methods succeed
+        # Mock that the package_json gem methods succeed
         allow(install_generator).to receive_messages(add_js_dependency: true, add_js_dependencies_batch: true,
                                                      install_js_dependencies: true)
       end
@@ -98,33 +97,6 @@
 
         # Verify state was set correctly to indicate package_json was used
         expect(install_generator.instance_variable_get(:@added_dependencies_to_package_json)).to be true
-        expect(install_generator.instance_variable_get(:@ran_direct_installs)).to be false
-      end
-    end
-
-    context "when falling back to direct npm commands" do
-      before do
-        allow(install_generator).to receive_messages(add_npm_dependencies: false, package_json_available?: false,
-                                                     package_json: nil)
-        # Mock File.exist? to not detect any lock files, forcing npm as default
-        allow(File).to receive(:exist?).and_call_original
-        allow(File).to receive(:exist?).with(File.join(install_generator.destination_root,
-                                                       "yarn.lock")).and_return(false)
-        allow(File).to receive(:exist?).with(File.join(install_generator.destination_root,
-                                                       "pnpm-lock.yaml")).and_return(false)
-        allow(File).to receive(:exist?).with(File.join(install_generator.destination_root,
-                                                       "package-lock.json")).and_return(false)
-        allow(File).to receive(:exist?).with(File.join(install_generator.destination_root,
-                                                       "package.json")).and_return(true)
-      end
-
-      it "runs individual installs plus final install" do
-        # Expect individual package installs plus one final bulk install
-        expect(install_generator).to receive(:system).with("npm", "install", anything).at_least(:once).and_return(true)
-        expect(install_generator).to receive(:system).with("npm", "install").once.and_return(true)
-
-        # Run the dependency setup
-        install_generator.send(:setup_js_dependencies)
       end
     end
   end