Fix duplicate dependency installation in install generator

ihabadham · claude · ihabadham · commit 5219afe0b602 · 2025-09-23T21:13:15.000+03:00
Root cause: JavaScript dependency methods in BaseGenerator were public, causing automatic execution when generators were invoked. This led to dependencies being installed twice when using --redux option. Changes: - Move dependency installation methods to private in BaseGenerator - Add explicit dependency management in InstallGenerator - Make Redux generator's success message method public - Ensure success messages appear correctly for all installation types Fixes duplicate installations while maintaining all existing functionality. All 51 generator tests pass. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
diff --git a/lib/generators/react_on_rails/base_generator.rb b/lib/generators/react_on_rails/base_generator.rb
@@ -95,45 +95,6 @@ def add_base_gems_to_gemfile
         run "bundle"
       end
 
-      def add_js_dependencies
-        add_react_on_rails_package
-        add_react_dependencies
-        add_css_dependencies
-        add_dev_dependencies
-        install_js_dependencies
-      end
-
-      private
-
-      def install_js_dependencies
-        # Detect which package manager to use
-        success = 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
-                  end
-
-        unless success
-          GeneratorMessages.add_warning(<<~MSG.strip)
-            ⚠️  JavaScript dependencies installation failed.
-
-            This could be due to network issues or missing package manager.
-            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 update_gitignore_for_auto_registration
         gitignore_path = File.join(destination_root, ".gitignore")
         return unless File.exist?(gitignore_path)
@@ -160,6 +121,28 @@ def append_to_spec_rails_helper
         end
       end
 
+      CONFIGURE_RSPEC_TO_COMPILE_ASSETS = <<-STR.strip_heredoc
+        RSpec.configure do |config|
+          # Ensure that if we are running js tests, we are using latest webpack assets
+          # This will use the defaults of :js and :server_rendering meta tags
+          ReactOnRails::TestHelper.configure_rspec_to_compile_assets(config)
+        end
+      STR
+
+      private
+
+      def setup_js_dependencies
+        add_js_dependencies
+        install_js_dependencies
+      end
+
+      def add_js_dependencies
+        add_react_on_rails_package
+        add_react_dependencies
+        add_css_dependencies
+        add_dev_dependencies
+      end
+
       def add_react_on_rails_package
         major_minor_patch_only = /\A\d+\.\d+\.\d+\z/
 
@@ -222,15 +205,34 @@ def add_dev_dependencies
         handle_npm_failure("development dependencies", dev_deps, dev: true) unless success
       end
 
-      CONFIGURE_RSPEC_TO_COMPILE_ASSETS = <<-STR.strip_heredoc
-        RSpec.configure do |config|
-          # Ensure that if we are running js tests, we are using latest webpack assets
-          # This will use the defaults of :js and :server_rendering meta tags
-          ReactOnRails::TestHelper.configure_rspec_to_compile_assets(config)
+      def install_js_dependencies
+        # Detect which package manager to use
+        success = 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
+                  end
+
+        unless success
+          GeneratorMessages.add_warning(<<~MSG.strip)
+            ⚠️  JavaScript dependencies installation failed.
+
+            This could be due to network issues or missing package manager.
+            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
-      STR
 
-      private
+        success
+      end
 
       def handle_npm_failure(dependency_type, packages, dev: false)
         install_command = dev ? "npm install --save-dev" : "npm install"
diff --git a/lib/generators/react_on_rails/install_generator.rb b/lib/generators/react_on_rails/install_generator.rb
@@ -79,6 +79,12 @@ def invoke_generators
         else
           invoke "react_on_rails:react_no_redux", [], { typescript: options.typescript? }
         end
+        setup_react_dependencies
+      end
+
+      def setup_react_dependencies
+        add_js_dependencies
+        install_js_dependencies
       end
 
       # NOTE: other requirements for existing files such as .gitignore or application.
@@ -412,6 +418,118 @@ def create_typescript_config
         puts Rainbow("✅ Created tsconfig.json").green
       end
 
+      def add_js_dependencies
+        add_react_on_rails_package
+        add_react_dependencies
+        add_css_dependencies
+        add_dev_dependencies
+      end
+
+      def add_react_on_rails_package
+        major_minor_patch_only = /\A\d+\.\d+\.\d+\z/
+
+        # Try to use package_json gem first, fall back to direct npm commands
+        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
+
+        puts "Installing React on Rails package..."
+        return if add_npm_dependencies(react_on_rails_pkg)
+
+        puts "Using direct npm commands as fallback"
+        success = system("npm", "install", *react_on_rails_pkg)
+        handle_npm_failure("react-on-rails package", react_on_rails_pkg) unless success
+      end
+
+      def add_react_dependencies
+        puts "Installing React dependencies..."
+        react_deps = %w[
+          react
+          react-dom
+          @babel/preset-react
+          prop-types
+          babel-plugin-transform-react-remove-prop-types
+          babel-plugin-macros
+        ]
+        return if add_npm_dependencies(react_deps)
+
+        success = system("npm", "install", *react_deps)
+        handle_npm_failure("React dependencies", react_deps) unless success
+      end
+
+      def add_css_dependencies
+        puts "Installing CSS handling dependencies..."
+        css_deps = %w[
+          css-loader
+          css-minimizer-webpack-plugin
+          mini-css-extract-plugin
+          style-loader
+        ]
+        return if add_npm_dependencies(css_deps)
+
+        success = system("npm", "install", *css_deps)
+        handle_npm_failure("CSS dependencies", css_deps) unless success
+      end
+
+      def add_dev_dependencies
+        puts "Installing development dependencies..."
+        dev_deps = %w[
+          @pmmmwh/react-refresh-webpack-plugin
+          react-refresh
+        ]
+        return if add_npm_dependencies(dev_deps, dev: true)
+
+        success = system("npm", "install", "--save-dev", *dev_deps)
+        handle_npm_failure("development dependencies", dev_deps, dev: true) unless success
+      end
+
+      def install_js_dependencies
+        # Detect which package manager to use
+        success = 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
+                  end
+
+        unless success
+          GeneratorMessages.add_warning(<<~MSG.strip)
+            ⚠️  JavaScript dependencies installation failed.
+
+            This could be due to network issues or missing package manager.
+            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 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
+
       # Removed: Shakapacker auto-installation logic (now explicit dependency)
 
       # Removed: Shakapacker 8+ is now required as explicit dependency
diff --git a/lib/generators/react_on_rails/react_with_redux_generator.rb b/lib/generators/react_on_rails/react_with_redux_generator.rb
@@ -89,6 +89,14 @@ def add_redux_npm_dependencies
         install_packages_with_fallback(regular_packages, dev: false, package_manager: package_manager)
       end
 
+      def add_redux_specific_messages
+        # Override the generic messages with Redux-specific instructions
+        GeneratorMessages.output.clear
+        GeneratorMessages.add_info(
+          GeneratorMessages.helpful_message_after_installation(component_name: "HelloWorldApp", route: "hello_world")
+        )
+      end
+
       private
 
       def install_packages_with_fallback(packages, dev:, package_manager:)
@@ -132,14 +140,6 @@ def dev_flag_for(package_manager)
         when "yarn", "bun" then "--dev"
         end
       end
-
-      def add_redux_specific_messages
-        # Override the generic messages with Redux-specific instructions
-        GeneratorMessages.output.clear
-        GeneratorMessages.add_info(
-          GeneratorMessages.helpful_message_after_installation(component_name: "HelloWorldApp", route: "hello_world")
-        )
-      end
     end
   end
 end
