Skip to content

Commit 4deda05

Browse files
refactor: Extract JS dependency management into shared module
Addresses code review feedback from PR #1788: 1. Created shared JsDependencyManager module to eliminate code duplication between base_generator.rb and install_generator.rb 2. Added comprehensive test coverage for: - Message deduplication (ensuring messages appear exactly once) - NPM install command deduplication - Proper module inclusion and method organization 3. Improved method organization in install_generator.rb This refactoring follows DRY principles and makes the codebase more maintainable by consolidating shared dependency management logic. Co-authored-by: Justin Gordon <[email protected]>
1 parent aa4f3c8 commit 4deda05

File tree

4 files changed

+302
-251
lines changed

4 files changed

+302
-251
lines changed

lib/generators/react_on_rails/base_generator.rb

Lines changed: 6 additions & 117 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,12 @@
44
require "fileutils"
55
require_relative "generator_messages"
66
require_relative "generator_helper"
7+
require_relative "js_dependency_manager"
78
module ReactOnRails
89
module Generators
910
class BaseGenerator < Rails::Generators::Base
1011
include GeneratorHelper
12+
include JsDependencyManager
1113

1214
Rails::Generators.hide_namespace(namespace)
1315
source_root(File.expand_path("templates", __dir__))
@@ -95,6 +97,10 @@ def add_base_gems_to_gemfile
9597
run "bundle"
9698
end
9799

100+
def add_js_dependencies
101+
setup_js_dependencies
102+
end
103+
98104
def update_gitignore_for_auto_registration
99105
gitignore_path = File.join(destination_root, ".gitignore")
100106
return unless File.exist?(gitignore_path)
@@ -134,123 +140,6 @@ def append_to_spec_rails_helper
134140

135141
private
136142

137-
def setup_js_dependencies
138-
add_js_dependencies
139-
install_js_dependencies
140-
end
141-
142-
def add_js_dependencies
143-
add_react_on_rails_package
144-
add_react_dependencies
145-
add_css_dependencies
146-
add_dev_dependencies
147-
end
148-
149-
def add_react_on_rails_package
150-
major_minor_patch_only = /\A\d+\.\d+\.\d+\z/
151-
152-
# Try to use package_json gem first, fall back to direct npm commands
153-
react_on_rails_pkg = if ReactOnRails::VERSION.match?(major_minor_patch_only)
154-
["react-on-rails@#{ReactOnRails::VERSION}"]
155-
else
156-
puts "Adding the latest react-on-rails NPM module. " \
157-
"Double check this is correct in package.json"
158-
["react-on-rails"]
159-
end
160-
161-
puts "Installing React on Rails package..."
162-
return if add_npm_dependencies(react_on_rails_pkg)
163-
164-
puts "Using direct npm commands as fallback"
165-
success = system("npm", "install", *react_on_rails_pkg)
166-
handle_npm_failure("react-on-rails package", react_on_rails_pkg) unless success
167-
end
168-
169-
def add_react_dependencies
170-
puts "Installing React dependencies..."
171-
react_deps = %w[
172-
react
173-
react-dom
174-
@babel/preset-react
175-
prop-types
176-
babel-plugin-transform-react-remove-prop-types
177-
babel-plugin-macros
178-
]
179-
return if add_npm_dependencies(react_deps)
180-
181-
success = system("npm", "install", *react_deps)
182-
handle_npm_failure("React dependencies", react_deps) unless success
183-
end
184-
185-
def add_css_dependencies
186-
puts "Installing CSS handling dependencies..."
187-
css_deps = %w[
188-
css-loader
189-
css-minimizer-webpack-plugin
190-
mini-css-extract-plugin
191-
style-loader
192-
]
193-
return if add_npm_dependencies(css_deps)
194-
195-
success = system("npm", "install", *css_deps)
196-
handle_npm_failure("CSS dependencies", css_deps) unless success
197-
end
198-
199-
def add_dev_dependencies
200-
puts "Installing development dependencies..."
201-
dev_deps = %w[
202-
@pmmmwh/react-refresh-webpack-plugin
203-
react-refresh
204-
]
205-
return if add_npm_dependencies(dev_deps, dev: true)
206-
207-
success = system("npm", "install", "--save-dev", *dev_deps)
208-
handle_npm_failure("development dependencies", dev_deps, dev: true) unless success
209-
end
210-
211-
def install_js_dependencies
212-
# Detect which package manager to use
213-
success = if File.exist?(File.join(destination_root, "yarn.lock"))
214-
system("yarn", "install")
215-
elsif File.exist?(File.join(destination_root, "pnpm-lock.yaml"))
216-
system("pnpm", "install")
217-
elsif File.exist?(File.join(destination_root, "package-lock.json")) ||
218-
File.exist?(File.join(destination_root, "package.json"))
219-
# Use npm for package-lock.json or as default fallback
220-
system("npm", "install")
221-
else
222-
true # No package manager detected, skip
223-
end
224-
225-
unless success
226-
GeneratorMessages.add_warning(<<~MSG.strip)
227-
⚠️ JavaScript dependencies installation failed.
228-
229-
This could be due to network issues or missing package manager.
230-
You can install dependencies manually later by running:
231-
• npm install (if using npm)
232-
• yarn install (if using yarn)
233-
• pnpm install (if using pnpm)
234-
MSG
235-
end
236-
237-
success
238-
end
239-
240-
def handle_npm_failure(dependency_type, packages, dev: false)
241-
install_command = dev ? "npm install --save-dev" : "npm install"
242-
GeneratorMessages.add_warning(<<~MSG.strip)
243-
⚠️ Failed to install #{dependency_type}.
244-
245-
The following packages could not be installed automatically:
246-
#{packages.map { |pkg| " • #{pkg}" }.join("\n")}
247-
248-
This could be due to network issues or missing package manager.
249-
You can install them manually later by running:
250-
#{install_command} #{packages.join(' ')}
251-
MSG
252-
end
253-
254143
def copy_webpack_main_config(base_path, config)
255144
webpack_config_path = "config/webpack/webpack.config.js"
256145

lib/generators/react_on_rails/install_generator.rb

Lines changed: 11 additions & 134 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,14 @@
44
require "json"
55
require_relative "generator_helper"
66
require_relative "generator_messages"
7+
require_relative "js_dependency_manager"
78

89
module ReactOnRails
910
module Generators
1011
# rubocop:disable Metrics/ClassLength
1112
class InstallGenerator < Rails::Generators::Base
1213
include GeneratorHelper
14+
include JsDependencyManager
1315

1416
# fetch USAGE file for details generator description
1517
source_root(File.expand_path(__dir__))
@@ -83,10 +85,7 @@ def invoke_generators
8385
end
8486

8587
def setup_react_dependencies
86-
@added_dependencies_to_package_json ||= false
87-
@ran_direct_installs ||= false
88-
add_js_dependencies
89-
install_js_dependencies if @added_dependencies_to_package_json && !@ran_direct_installs
88+
setup_js_dependencies
9089
end
9190

9291
# NOTE: other requirements for existing files such as .gitignore or application.
@@ -346,11 +345,17 @@ def install_typescript_dependencies
346345
]
347346

348347
# Try using GeneratorHelper first (package manager agnostic)
349-
return if add_npm_dependencies(typescript_packages, dev: true)
348+
if add_npm_dependencies(typescript_packages, dev: true)
349+
@added_dependencies_to_package_json = true
350+
return
351+
end
350352

351353
# Fallback to npm if GeneratorHelper fails
352354
success = system("npm", "install", "--save-dev", *typescript_packages)
353-
return if success
355+
if success
356+
@ran_direct_installs = true
357+
return
358+
end
354359

355360
warning = <<~MSG.strip
356361
⚠️ Failed to install TypeScript dependencies automatically.
@@ -420,134 +425,6 @@ def create_typescript_config
420425
puts Rainbow("✅ Created tsconfig.json").green
421426
end
422427

423-
def add_js_dependencies
424-
add_react_on_rails_package
425-
add_react_dependencies
426-
add_css_dependencies
427-
add_dev_dependencies
428-
end
429-
430-
def add_react_on_rails_package
431-
major_minor_patch_only = /\A\d+\.\d+\.\d+\z/
432-
433-
# Try to use package_json gem first, fall back to direct npm commands
434-
react_on_rails_pkg = if ReactOnRails::VERSION.match?(major_minor_patch_only)
435-
["react-on-rails@#{ReactOnRails::VERSION}"]
436-
else
437-
puts "Adding the latest react-on-rails NPM module. " \
438-
"Double check this is correct in package.json"
439-
["react-on-rails"]
440-
end
441-
442-
puts "Installing React on Rails package..."
443-
if add_npm_dependencies(react_on_rails_pkg)
444-
@added_dependencies_to_package_json = true
445-
return
446-
end
447-
448-
puts "Using direct npm commands as fallback"
449-
success = system("npm", "install", *react_on_rails_pkg)
450-
@ran_direct_installs = true if success
451-
handle_npm_failure("react-on-rails package", react_on_rails_pkg) unless success
452-
end
453-
454-
def add_react_dependencies
455-
puts "Installing React dependencies..."
456-
react_deps = %w[
457-
react
458-
react-dom
459-
@babel/preset-react
460-
prop-types
461-
babel-plugin-transform-react-remove-prop-types
462-
babel-plugin-macros
463-
]
464-
if add_npm_dependencies(react_deps)
465-
@added_dependencies_to_package_json = true
466-
return
467-
end
468-
469-
success = system("npm", "install", *react_deps)
470-
@ran_direct_installs = true if success
471-
handle_npm_failure("React dependencies", react_deps) unless success
472-
end
473-
474-
def add_css_dependencies
475-
puts "Installing CSS handling dependencies..."
476-
css_deps = %w[
477-
css-loader
478-
css-minimizer-webpack-plugin
479-
mini-css-extract-plugin
480-
style-loader
481-
]
482-
if add_npm_dependencies(css_deps)
483-
@added_dependencies_to_package_json = true
484-
return
485-
end
486-
487-
success = system("npm", "install", *css_deps)
488-
@ran_direct_installs = true if success
489-
handle_npm_failure("CSS dependencies", css_deps) unless success
490-
end
491-
492-
def add_dev_dependencies
493-
puts "Installing development dependencies..."
494-
dev_deps = %w[
495-
@pmmmwh/react-refresh-webpack-plugin
496-
react-refresh
497-
]
498-
if add_npm_dependencies(dev_deps, dev: true)
499-
@added_dependencies_to_package_json = true
500-
return
501-
end
502-
503-
success = system("npm", "install", "--save-dev", *dev_deps)
504-
@ran_direct_installs = true if success
505-
handle_npm_failure("development dependencies", dev_deps, dev: true) unless success
506-
end
507-
508-
def install_js_dependencies
509-
# Detect which package manager to use
510-
success = if File.exist?(File.join(destination_root, "yarn.lock"))
511-
system("yarn", "install")
512-
elsif File.exist?(File.join(destination_root, "pnpm-lock.yaml"))
513-
system("pnpm", "install")
514-
elsif File.exist?(File.join(destination_root, "package-lock.json")) ||
515-
File.exist?(File.join(destination_root, "package.json"))
516-
# Use npm for package-lock.json or as default fallback
517-
system("npm", "install")
518-
else
519-
true # No package manager detected, skip
520-
end
521-
522-
unless success
523-
GeneratorMessages.add_warning(<<~MSG.strip)
524-
⚠️ JavaScript dependencies installation failed.
525-
526-
This could be due to network issues or missing package manager.
527-
You can install dependencies manually later by running:
528-
• npm install (if using npm)
529-
• yarn install (if using yarn)
530-
• pnpm install (if using pnpm)
531-
MSG
532-
end
533-
534-
success
535-
end
536-
537-
def handle_npm_failure(dependency_type, packages, dev: false)
538-
install_command = dev ? "npm install --save-dev" : "npm install"
539-
GeneratorMessages.add_warning(<<~MSG.strip)
540-
⚠️ Failed to install #{dependency_type}.
541-
542-
The following packages could not be installed automatically:
543-
#{packages.map { |pkg| " • #{pkg}" }.join("\n")}
544-
545-
This could be due to network issues or missing package manager.
546-
You can install them manually later by running:
547-
#{install_command} #{packages.join(' ')}
548-
MSG
549-
end
550-
551428
# Removed: Shakapacker auto-installation logic (now explicit dependency)
552429

553430
# Removed: Shakapacker 8+ is now required as explicit dependency

0 commit comments

Comments
 (0)