Skip to content

Commit 29f2b61

Browse files
justin808claude
andcommitted
fix: Address code review issues in JS dependency manager
- Restore version validation for pre-release versions in add_react_on_rails_package - Clarify TypeScript dependencies comment for SWC-first approach - Add explanatory comment about package_json availability - Fix test mocks to match actual implementation using add_npm_dependencies - Remove stale react-on-rails@^16.1.0 entry from yarn.lock 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 62d06f0 commit 29f2b61

File tree

3 files changed

+30
-13
lines changed

3 files changed

+30
-13
lines changed

lib/generators/react_on_rails/js_dependency_manager.rb

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -78,9 +78,10 @@ module JsDependencyManager
7878

7979
# TypeScript dependencies (only installed when --typescript flag is used)
8080
# Note: @babel/preset-typescript is NOT included because:
81-
# - SWC (the default) has built-in TypeScript support
82-
# - Shakapacker handles the loader configuration
83-
# - For Babel users, they can add @babel/preset-typescript manually if needed
81+
# - SWC is now the default webpack_loader (has built-in TypeScript support)
82+
# - Shakapacker handles the loader configuration via shakapacker.yml
83+
# - If users choose webpack_loader: 'babel', they should manually add @babel/preset-typescript
84+
# and configure it in their babel.config.js
8485
TYPESCRIPT_DEPENDENCIES = %w[
8586
typescript
8687
@types/react
@@ -114,8 +115,17 @@ def add_js_dependencies
114115
end
115116

116117
def add_react_on_rails_package
117-
# Always use exact version match between gem and npm package
118-
react_on_rails_pkg = "react-on-rails@#{ReactOnRails::VERSION}"
118+
# Use exact version match between gem and npm package for stable releases
119+
# For pre-release versions (e.g., 16.1.0-rc.1), use latest to avoid installing
120+
# a version that may not exist in the npm registry
121+
major_minor_patch_only = /\A\d+\.\d+\.\d+\z/
122+
react_on_rails_pkg = if ReactOnRails::VERSION.match?(major_minor_patch_only)
123+
"react-on-rails@#{ReactOnRails::VERSION}"
124+
else
125+
puts "Adding the latest react-on-rails NPM module. " \
126+
"Double check this is correct in package.json"
127+
"react-on-rails"
128+
end
119129

120130
puts "Installing React on Rails package..."
121131
if add_js_dependency(react_on_rails_pkg)
@@ -237,6 +247,10 @@ def add_js_dependencies_batch(packages, dev: false)
237247

238248
def install_js_dependencies
239249
# Use package_json gem's install method (always available via shakapacker)
250+
# package_json is guaranteed to be available because:
251+
# 1. react_on_rails gemspec requires shakapacker
252+
# 2. shakapacker gemspec requires package_json
253+
# 3. GeneratorHelper provides package_json method
240254
package_json.manager.install
241255
true
242256
rescue StandardError => e

spec/react_on_rails/generators/message_deduplication_spec.rb

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,9 +83,17 @@
8383

8484
context "when using package_json gem (always available via shakapacker)" do
8585
before do
86-
# Mock that the package_json gem methods succeed
87-
allow(install_generator).to receive_messages(add_js_dependency: true, add_js_dependencies_batch: true,
88-
install_js_dependencies: true)
86+
# Mock the actual methods used by JsDependencyManager
87+
# add_npm_dependencies is from GeneratorHelper and is used by add_js_dependencies_batch
88+
# Mock package_json to prevent actual package manager calls
89+
# rubocop:disable RSpec/VerifiedDoubles
90+
mock_manager = double("PackageManager", install: true)
91+
mock_package_json = double("PackageJson", manager: mock_manager)
92+
# rubocop:enable RSpec/VerifiedDoubles
93+
allow(install_generator).to receive_messages(
94+
add_npm_dependencies: true,
95+
package_json: mock_package_json
96+
)
8997
end
9098

9199
it "does not run duplicate install commands" do

yarn.lock

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5364,11 +5364,6 @@ react-on-rails-rsc@19.0.2:
53645364
neo-async "^2.6.1"
53655365
webpack-sources "^3.2.0"
53665366

5367-
react-on-rails@^16.1.0:
5368-
version "16.1.0"
5369-
resolved "https://registry.yarnpkg.com/react-on-rails/-/react-on-rails-16.1.0.tgz#1d6a91edf6c5675f22f4449483b604690471a76e"
5370-
integrity sha512-EZbs868+V6gkxY52jeim64oETHSg6ztym5aL9FFBY5uEadb/FaAAoCNrutyObFR/XgzGqmeSKQdW1DvPl8BVyA==
5371-
53725367
react@^19.0.0:
53735368
version "19.0.0"
53745369
resolved "https://registry.npmjs.org/react/-/react-19.0.0.tgz"

0 commit comments

Comments
 (0)