Skip to content

Commit 3d89001

Browse files
justin808claude
andcommitted
fix: Improve JsDependencyManager robustness and consistency
Address code review feedback to enhance error handling and consistency: - Add exact: true flag to add_package method for consistency with add_npm_dependencies - Add nil check for package_json in install_js_dependencies to prevent nil pointer errors - Remove orphaned comment at end of module - Update tests to verify exact: true flag is passed correctly - Add test coverage for nil package_json scenario in install_js_dependencies These changes improve code robustness while maintaining backward compatibility. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 6c6e1eb commit 3d89001

File tree

2 files changed

+28
-9
lines changed

2 files changed

+28
-9
lines changed

lib/generators/react_on_rails/js_dependency_manager.rb

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -279,9 +279,9 @@ def add_package(package, dev: false)
279279
# Ensure package is in array format for package_json gem
280280
packages_array = [package]
281281
if dev
282-
pj.manager.add(packages_array, type: :dev)
282+
pj.manager.add(packages_array, type: :dev, exact: true)
283283
else
284-
pj.manager.add(packages_array)
284+
pj.manager.add(packages_array, exact: true)
285285
end
286286
true
287287
rescue StandardError => e
@@ -309,7 +309,13 @@ def install_js_dependencies
309309
# 1. react_on_rails gemspec requires shakapacker
310310
# 2. shakapacker gemspec requires package_json
311311
# 3. GeneratorHelper provides package_json method
312-
package_json.manager.install
312+
pj = package_json
313+
unless pj
314+
GeneratorMessages.add_warning("package_json not available, skipping dependency installation")
315+
return false
316+
end
317+
318+
pj.manager.install
313319
true
314320
rescue StandardError => e
315321
GeneratorMessages.add_warning(<<~MSG.strip)
@@ -323,8 +329,6 @@ def install_js_dependencies
323329
MSG
324330
false
325331
end
326-
327-
# No longer needed since package_json gem handles package manager detection
328332
end
329333
end
330334
end

spec/react_on_rails/generators/js_dependency_manager_spec.rb

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -142,13 +142,18 @@ def errors
142142
it "adds a single package successfully" do
143143
result = instance.send(:add_package, "react-on-rails@16.0.0")
144144
expect(result).to be(true)
145-
expect(mock_manager).to have_received(:add).with(["react-on-rails@16.0.0"])
145+
expect(mock_manager).to have_received(:add).with(["react-on-rails@16.0.0"], exact: true)
146146
end
147147

148148
it "adds a dev dependency when dev: true" do
149149
result = instance.send(:add_package, "typescript", dev: true)
150150
expect(result).to be(true)
151-
expect(mock_manager).to have_received(:add).with(["typescript"], type: :dev)
151+
expect(mock_manager).to have_received(:add).with(["typescript"], type: :dev, exact: true)
152+
end
153+
154+
it "uses exact: true flag for consistency with add_npm_dependencies" do
155+
instance.send(:add_package, "some-package")
156+
expect(mock_manager).to have_received(:add).with(["some-package"], exact: true)
152157
end
153158

154159
it "returns false when package_json is nil" do
@@ -171,6 +176,16 @@ def errors
171176
expect(mock_manager).to have_received(:install)
172177
end
173178

179+
it "returns false and adds warning when package_json is nil" do
180+
instance.package_json = nil
181+
182+
result = instance.send(:install_js_dependencies)
183+
184+
expect(result).to be(false)
185+
expect(warnings.size).to be > 0
186+
expect(warnings.first.to_s).to include("package_json not available")
187+
end
188+
174189
it "returns false and adds warning when install fails" do
175190
allow(mock_manager).to receive(:install).and_raise(StandardError, "Network timeout")
176191

@@ -190,13 +205,13 @@ def errors
190205

191206
it "adds react-on-rails with version for stable releases" do
192207
instance.send(:add_react_on_rails_package)
193-
expect(mock_manager).to have_received(:add).with(["react-on-rails@16.0.0"])
208+
expect(mock_manager).to have_received(:add).with(["react-on-rails@16.0.0"], exact: true)
194209
end
195210

196211
it "adds react-on-rails without version for pre-releases" do
197212
stub_const("ReactOnRails::VERSION", "16.0.0-rc.1")
198213
instance.send(:add_react_on_rails_package)
199-
expect(mock_manager).to have_received(:add).with(["react-on-rails"])
214+
expect(mock_manager).to have_received(:add).with(["react-on-rails"], exact: true)
200215
end
201216

202217
it "adds warning when add_package fails" do

0 commit comments

Comments
 (0)