Skip to content

Commit aa4f3c8

Browse files
authored
Fix duplicate messages in install generator (#1788)
Key improvements: - Fixes duplicate Redux success messages appearing twice - Prevents NPM install commands from executing twice with duplicate output - Makes dependency installation methods private in BaseGenerator - Tracks package_json vs direct install usage to avoid duplicate installations - Preserves Shakapacker warnings and installation messages properly - Redux generator now appends messages instead of clearing previous output Breaking changes: None Security implications: None Impact: - Existing installations: No impact, generator only runs during initial setup - New installations: Cleaner output without duplicate messages and faster install - Generator test suite fully passes with 51 tests confirming proper behavior
1 parent ce57aed commit aa4f3c8

File tree

3 files changed

+194
-52
lines changed

3 files changed

+194
-52
lines changed

lib/generators/react_on_rails/base_generator.rb

Lines changed: 48 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -95,42 +95,6 @@ def add_base_gems_to_gemfile
9595
run "bundle"
9696
end
9797

98-
def add_js_dependencies
99-
add_react_on_rails_package
100-
add_react_dependencies
101-
add_css_dependencies
102-
add_dev_dependencies
103-
end
104-
105-
def install_js_dependencies
106-
# Detect which package manager to use
107-
success = if File.exist?(File.join(destination_root, "yarn.lock"))
108-
system("yarn", "install")
109-
elsif File.exist?(File.join(destination_root, "pnpm-lock.yaml"))
110-
system("pnpm", "install")
111-
elsif File.exist?(File.join(destination_root, "package-lock.json")) ||
112-
File.exist?(File.join(destination_root, "package.json"))
113-
# Use npm for package-lock.json or as default fallback
114-
system("npm", "install")
115-
else
116-
true # No package manager detected, skip
117-
end
118-
119-
unless success
120-
GeneratorMessages.add_warning(<<~MSG.strip)
121-
⚠️ JavaScript dependencies installation failed.
122-
123-
This could be due to network issues or missing package manager.
124-
You can install dependencies manually later by running:
125-
• npm install (if using npm)
126-
• yarn install (if using yarn)
127-
• pnpm install (if using pnpm)
128-
MSG
129-
end
130-
131-
success
132-
end
133-
13498
def update_gitignore_for_auto_registration
13599
gitignore_path = File.join(destination_root, ".gitignore")
136100
return unless File.exist?(gitignore_path)
@@ -160,6 +124,28 @@ def append_to_spec_rails_helper
160124
end
161125
end
162126

127+
CONFIGURE_RSPEC_TO_COMPILE_ASSETS = <<-STR.strip_heredoc
128+
RSpec.configure do |config|
129+
# Ensure that if we are running js tests, we are using latest webpack assets
130+
# This will use the defaults of :js and :server_rendering meta tags
131+
ReactOnRails::TestHelper.configure_rspec_to_compile_assets(config)
132+
end
133+
STR
134+
135+
private
136+
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+
163149
def add_react_on_rails_package
164150
major_minor_patch_only = /\A\d+\.\d+\.\d+\z/
165151

@@ -222,15 +208,34 @@ def add_dev_dependencies
222208
handle_npm_failure("development dependencies", dev_deps, dev: true) unless success
223209
end
224210

225-
CONFIGURE_RSPEC_TO_COMPILE_ASSETS = <<-STR.strip_heredoc
226-
RSpec.configure do |config|
227-
# Ensure that if we are running js tests, we are using latest webpack assets
228-
# This will use the defaults of :js and :server_rendering meta tags
229-
ReactOnRails::TestHelper.configure_rspec_to_compile_assets(config)
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
230235
end
231-
STR
232236

233-
private
237+
success
238+
end
234239

235240
def handle_npm_failure(dependency_type, packages, dev: false)
236241
install_command = dev ? "npm install --save-dev" : "npm install"

lib/generators/react_on_rails/install_generator.rb

Lines changed: 139 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,9 @@ def run_generators
4040
if installation_prerequisites_met? || options.ignore_warnings?
4141
invoke_generators
4242
add_bin_scripts
43-
add_post_install_message
43+
# Only add the post install message if not using Redux
44+
# Redux generator handles its own messages
45+
add_post_install_message unless options.redux?
4446
else
4547
error = <<~MSG.strip
4648
🚫 React on Rails generator prerequisites not met!
@@ -77,6 +79,14 @@ def invoke_generators
7779
else
7880
invoke "react_on_rails:react_no_redux", [], { typescript: options.typescript? }
7981
end
82+
setup_react_dependencies
83+
end
84+
85+
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
8090
end
8191

8292
# NOTE: other requirements for existing files such as .gitignore or application.
@@ -410,6 +420,134 @@ def create_typescript_config
410420
puts Rainbow("✅ Created tsconfig.json").green
411421
end
412422

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+
413551
# Removed: Shakapacker auto-installation logic (now explicit dependency)
414552

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

lib/generators/react_on_rails/react_with_redux_generator.rb

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,13 @@ def add_redux_npm_dependencies
8989
install_packages_with_fallback(regular_packages, dev: false, package_manager: package_manager)
9090
end
9191

92+
def add_redux_specific_messages
93+
# Append Redux-specific post-install instructions
94+
GeneratorMessages.add_info(
95+
GeneratorMessages.helpful_message_after_installation(component_name: "HelloWorldApp", route: "hello_world")
96+
)
97+
end
98+
9299
private
93100

94101
def install_packages_with_fallback(packages, dev:, package_manager:)
@@ -132,14 +139,6 @@ def dev_flag_for(package_manager)
132139
when "yarn", "bun" then "--dev"
133140
end
134141
end
135-
136-
def add_redux_specific_messages
137-
# Override the generic messages with Redux-specific instructions
138-
GeneratorMessages.output.clear
139-
GeneratorMessages.add_info(
140-
GeneratorMessages.helpful_message_after_installation(component_name: "HelloWorldApp", route: "hello_world")
141-
)
142-
end
143142
end
144143
end
145144
end

0 commit comments

Comments
 (0)