Skip to content

Commit d988757

Browse files
justin808claude
andcommitted
fix: Add Rspack and TypeScript support to shared JS dependency manager
This commit addresses critical issues identified in code review: ### 1. Added Missing Rspack Support - **Problem**: Rspack dependencies were lost during the refactor to shared module - **Solution**: Added RSPACK_DEPENDENCIES and RSPACK_DEV_DEPENDENCIES constants - **Implementation**: - Created `add_rspack_dependencies` method in shared module - Modified `add_dev_dependencies` to use Rspack-specific dev deps when `options.rspack?` is true - Conditional installation in `add_js_dependencies` checks for `options.rspack?` - **Impact**: Users with --rspack flag now get proper Rspack packages installed ### 2. Fixed TypeScript Dependency Consistency - **Problem**: TypeScript dependency handling was inconsistent with shared module pattern - **Solution**: Extracted TypeScript dependencies into shared module - **Implementation**: - Added TYPESCRIPT_DEPENDENCIES constant - Created `add_typescript_dependencies` method following same pattern as other deps - Updated `install_generator.rb` to delegate to shared method - Removed manual instance variable setting - **Impact**: Consistent error handling and pattern across all dependency types ### 3. Documented Unconditional Install Behavior - **Problem**: Unclear why `install_js_dependencies` is called unconditionally - **Solution**: Added comprehensive documentation explaining the design decision - **Rationale**: - package_json gem's install is idempotent - only installs what's needed - Prevents edge cases where package.json modified but dependencies not installed - Safer and simpler than conditional logic that could miss cases - **Impact**: Future maintainers understand why the unconditional call is correct ### 4. Enhanced Module Documentation - **Updated Instance Variables section**: Clarified module initializes variables - **Added Optional Methods section**: Documents options.rspack? and options.typescript? - **Added Installation Behavior section**: Explains idempotent install behavior - **Impact**: Clear contract for classes including the module ### 5. Constants Usage Verification - Verified all constants (REACT_DEPENDENCIES, CSS_DEPENDENCIES, DEV_DEPENDENCIES) are properly used - All dependency methods use their corresponding constants correctly ### Testing - ✅ All tests pass (6 examples, 0 failures) - ✅ bundle exec rubocop passes with zero offenses - ✅ Git hooks (pre-commit, pre-push) verified clean ### Files Changed - `lib/generators/react_on_rails/js_dependency_manager.rb`: Added Rspack, TypeScript support, enhanced docs - `lib/generators/react_on_rails/install_generator.rb`: Simplified TypeScript method to delegate to shared module 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
1 parent 74b9038 commit d988757

File tree

2 files changed

+80
-22
lines changed

2 files changed

+80
-22
lines changed

lib/generators/react_on_rails/install_generator.rb

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -365,23 +365,8 @@ def missing_package_manager?
365365

366366
def install_typescript_dependencies
367367
puts Rainbow("📝 Installing TypeScript dependencies...").yellow
368-
369-
# Install TypeScript and React type definitions
370-
typescript_packages = %w[
371-
typescript
372-
@types/react
373-
@types/react-dom
374-
@babel/preset-typescript
375-
]
376-
377-
# Use the shared dependency management approach via GeneratorHelper
378-
unless add_npm_dependencies(typescript_packages, dev: true)
379-
# This should not happen since package_json is always available via shakapacker
380-
raise "Failed to add TypeScript dependencies (#{typescript_packages.join(', ')}) via package_json gem. " \
381-
"This indicates shakapacker dependency may not be properly installed."
382-
end
383-
384-
@added_dependencies_to_package_json = true
368+
# Delegate to shared module for consistent dependency management
369+
add_typescript_dependencies
385370
end
386371

387372
def create_css_module_types

lib/generators/react_on_rails/js_dependency_manager.rb

Lines changed: 78 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,16 +11,28 @@ module Generators
1111
# Since react_on_rails requires shakapacker, and shakapacker includes
1212
# package_json as a dependency, the package_json gem is always available.
1313
#
14-
# == Required Instance Variables
15-
# Including classes must support these instance variables:
14+
# == Instance Variables
15+
# The module initializes and manages these instance variables:
1616
# - @added_dependencies_to_package_json: Boolean tracking if package_json gem was used
17+
# (initialized by setup_js_dependencies using `unless defined?` pattern)
1718
#
1819
# == Required Methods
1920
# Including classes must include GeneratorHelper module which provides:
2021
# - add_npm_dependencies(packages, dev: false): Add packages via package_json gem
2122
# - package_json: Access to PackageJson instance (always available via shakapacker)
2223
# - destination_root: Generator destination directory
2324
#
25+
# == Optional Methods
26+
# Including classes may define:
27+
# - options.rspack?: Returns true if --rspack flag is set (for Rspack support)
28+
# - options.typescript?: Returns true if --typescript flag is set (for TypeScript support)
29+
#
30+
# == Installation Behavior
31+
# The module ALWAYS runs package manager install after adding dependencies.
32+
# This is safe because package_json gem's install is idempotent - it only
33+
# installs what's actually needed from package.json. This prevents edge cases
34+
# where package.json was modified but dependencies weren't installed.
35+
#
2436
# == Usage
2537
# Include this module in generator classes and call setup_js_dependencies
2638
# to handle all JS dependency installation via package_json gem.
@@ -43,26 +55,56 @@ module JsDependencyManager
4355
style-loader
4456
].freeze
4557

46-
# Development-only dependencies for hot reloading
58+
# Development-only dependencies for hot reloading (Webpack)
4759
DEV_DEPENDENCIES = %w[
4860
@pmmmwh/react-refresh-webpack-plugin
4961
react-refresh
5062
].freeze
5163

64+
# Rspack core dependencies (only installed when --rspack flag is used)
65+
RSPACK_DEPENDENCIES = %w[
66+
@rspack/core
67+
rspack-manifest-plugin
68+
].freeze
69+
70+
# Rspack development dependencies for hot reloading
71+
RSPACK_DEV_DEPENDENCIES = %w[
72+
@rspack/cli
73+
@rspack/plugin-react-refresh
74+
react-refresh
75+
].freeze
76+
77+
# TypeScript dependencies (only installed when --typescript flag is used)
78+
TYPESCRIPT_DEPENDENCIES = %w[
79+
typescript
80+
@types/react
81+
@types/react-dom
82+
@babel/preset-typescript
83+
].freeze
84+
5285
private
5386

5487
def setup_js_dependencies
5588
# Initialize instance variable if not already defined by including class
5689
# This ensures safe operation when the module is first included
5790
@added_dependencies_to_package_json = false unless defined?(@added_dependencies_to_package_json)
5891
add_js_dependencies
92+
93+
# Always run install to ensure all dependencies are properly installed.
94+
# The package_json gem's install method is idempotent and safe to call
95+
# even if packages were already added - it will only install what's needed.
96+
# This ensures edge cases where package.json was modified but install wasn't
97+
# run are handled correctly.
5998
install_js_dependencies
6099
end
61100

62101
def add_js_dependencies
63102
add_react_on_rails_package
64103
add_react_dependencies
65104
add_css_dependencies
105+
# Rspack dependencies are only added when --rspack flag is used
106+
add_rspack_dependencies if respond_to?(:options) && options.rspack?
107+
# Dev dependencies vary based on bundler choice
66108
add_dev_dependencies
67109
end
68110

@@ -104,14 +146,45 @@ def add_css_dependencies
104146
end
105147
end
106148

149+
def add_rspack_dependencies
150+
puts "Installing Rspack core dependencies..."
151+
152+
if add_js_dependencies_batch(RSPACK_DEPENDENCIES)
153+
@added_dependencies_to_package_json = true
154+
else
155+
# This should not happen since package_json is always available via shakapacker
156+
raise "Failed to add Rspack dependencies (#{RSPACK_DEPENDENCIES.join(', ')}) via package_json gem. " \
157+
"This indicates shakapacker dependency may not be properly installed."
158+
end
159+
end
160+
161+
def add_typescript_dependencies
162+
puts "Installing TypeScript dependencies..."
163+
164+
if add_js_dependencies_batch(TYPESCRIPT_DEPENDENCIES, dev: true)
165+
@added_dependencies_to_package_json = true
166+
else
167+
# This should not happen since package_json is always available via shakapacker
168+
raise "Failed to add TypeScript dependencies (#{TYPESCRIPT_DEPENDENCIES.join(', ')}) via package_json gem. " \
169+
"This indicates shakapacker dependency may not be properly installed."
170+
end
171+
end
172+
107173
def add_dev_dependencies
108174
puts "Installing development dependencies..."
109175

110-
if add_js_dependencies_batch(DEV_DEPENDENCIES, dev: true)
176+
# Use Rspack-specific dev dependencies if --rspack flag is set
177+
dev_deps = if respond_to?(:options) && options.rspack?
178+
RSPACK_DEV_DEPENDENCIES
179+
else
180+
DEV_DEPENDENCIES
181+
end
182+
183+
if add_js_dependencies_batch(dev_deps, dev: true)
111184
@added_dependencies_to_package_json = true
112185
else
113186
# This should not happen since package_json is always available via shakapacker
114-
raise "Failed to add development dependencies (#{DEV_DEPENDENCIES.join(', ')}) via package_json gem. " \
187+
raise "Failed to add development dependencies (#{dev_deps.join(', ')}) via package_json gem. " \
115188
"This indicates shakapacker dependency may not be properly installed."
116189
end
117190
end

0 commit comments

Comments
 (0)