Skip to content

Commit 463a31f

Browse files
justin808claude
andauthored
Fix beta/RC version handling in generator (#2066)
## Summary Fixes version mismatch errors when using beta, RC, or alpha versions of react_on_rails by updating the version detection regex in the generator. ## Problem When running the generator with a pre-release version (e.g., `16.2.0-beta.10`), the version check regex only matched stable versions like `16.2.0`. This caused the generator to install `react-on-rails` (latest stable from npm) instead of `[email protected]`, leading to version mismatch errors: ``` Package: 16.1.2 (latest stable from npm) Gem: 16.2.0.beta.10 (current beta version) ``` ## Root Cause In `lib/generators/react_on_rails/js_dependency_manager.rb`, the regex was: ```ruby major_minor_patch_only = /\A\d+\.\d+\.\d+\z/ ``` This only matched stable versions (e.g., `16.2.0`) and rejected pre-release versions (e.g., `16.2.0-beta.10`). When the regex didn't match, the code would install `"react-on-rails"` without a version specifier, which installs the latest stable version from npm instead of the beta version. ## Solution Updated the regex to also match pre-release versions: ```ruby version_with_optional_prerelease = /\A\d+\.\d+\.\d+(-[a-zA-Z0-9.]+)?\z/ ``` This now matches: - ✅ Stable: `16.2.0` - ✅ Beta: `16.2.0-beta.10` - ✅ RC: `16.1.0-rc.1` - ✅ Alpha: `16.0.0-alpha.5` - ✅ Complex pre-releases: `1.2.3-pre.release.1` ## Changes - Updated regex to include optional pre-release identifier: `(-[a-zA-Z0-9.]+)?` - Improved comments explaining what versions are matched and why - Enhanced warning message when version format is unrecognized ## Testing Verified the regex matches all expected version formats: - Stable versions: `16.2.0`, `16.1.2` - Beta versions: `16.2.0-beta.10` - RC versions: `16.1.0-rc.1` - Alpha versions: `16.0.0-alpha.5` - Invalid formats correctly rejected: `invalid`, `16.2`, `16.2.0.1` ## Impact This fix ensures that: - Beta/RC versions work correctly during development and CI - The exact gem version matches the exact npm package version - No more version mismatch errors when using pre-release versions - Example generation in CI passes with beta versions ## Related This fixes the CI failure shown in the error log where shakapacker examples failed with version mismatch errors during beta releases. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added an alias command (/run-skipped-tests) that triggers the same test run behavior as /run-skipped-ci. * **Bug Fixes** * Enhanced version parsing so pre-release react-on-rails versions (alpha, beta, rc) are recognized and pinned correctly; clearer warning for unrecognized formats. * **Documentation** * Updated docs and CI help/examples to include the new alias and clarify usage. * **Tests** * Expanded tests to cover pre-release version handling and invalid-version warnings. <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub> <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude <[email protected]>
1 parent ac0d175 commit 463a31f

File tree

3 files changed

+83
-8
lines changed

3 files changed

+83
-8
lines changed

analysis/rake-task-duplicate-analysis.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,13 @@ Rails Engines have **two different mechanisms** for loading rake tasks, and this
3131
2. **Manual Loading (Railtie Layer)**: The `rake_tasks` block explicitly loads specific files
3232

3333
Because the task files existed in `lib/tasks/`:
34+
3435
- `lib/tasks/assets.rake`
3536
- `lib/tasks/generate_packs.rake`
3637
- `lib/tasks/locale.rake`
3738

3839
They were being loaded **twice**:
40+
3941
- Once automatically by Rails::Engine from the `lib/tasks/` directory
4042
- Once explicitly by the `rake_tasks` block
4143

@@ -69,11 +71,13 @@ Based on the PR context and commit message, the most likely reasons:
6971
## The Impact
7072

7173
Tasks affected by duplicate execution:
74+
7275
- `react_on_rails:assets:webpack` - Webpack builds ran twice
7376
- `react_on_rails:generate_packs` - Pack generation ran twice
7477
- `react_on_rails:locale` - Locale file generation ran twice
7578

7679
This meant:
80+
7781
- **2x build times** during asset precompilation
7882
- **Slower CI** builds
7983
- **Confusing console output** showing duplicate webpack compilation messages
@@ -98,6 +102,7 @@ end
98102
## Key Lesson
99103

100104
**Rails::Engine Best Practice**: If your rake task files are in `lib/tasks/`, you don't need a `rake_tasks` block. Rails will load them automatically. Only use `rake_tasks do` if:
105+
101106
- Tasks are in a non-standard location
102107
- You need to programmatically generate tasks
103108
- You need to pass context to the tasks
@@ -112,6 +117,7 @@ end
112117
### For Code Reviews
113118

114119
This incident highlights the challenge of reviewing massive PRs:
120+
115121
- **97 files changed** made it nearly impossible to catch subtle issues
116122
- The `rake_tasks` addition was 6 lines in a file that wasn't the focus of the PR
117123
- The duplicate loading bug only manifested during asset precompilation, not during normal development
@@ -120,6 +126,7 @@ This incident highlights the challenge of reviewing massive PRs:
120126
### For Testing
121127

122128
The duplicate execution bug was subtle:
129+
123130
- **Didn't cause failures**—just slower builds (2x time)
124131
- **Hard to notice locally**—developers might not realize builds were taking twice as long
125132
- **Only obvious in CI**—where build times are closely monitored
@@ -128,6 +135,7 @@ The duplicate execution bug was subtle:
128135
### For Documentation
129136

130137
Better documentation of Rails::Engine automatic loading would help:
138+
131139
- Many Rails guides show `rake_tasks` blocks without mentioning automatic loading
132140
- The Rails Engine guide doesn't clearly state when NOT to use `rake_tasks`
133141
- This leads to cargo-culting of the pattern

lib/generators/react_on_rails/js_dependency_manager.rb

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -123,14 +123,36 @@ def add_js_dependencies
123123
end
124124

125125
def add_react_on_rails_package
126-
# Use exact version match between gem and npm package for stable releases
127-
# For pre-release versions (e.g., 16.1.0-rc.1), use latest to avoid installing
128-
# a version that may not exist in the npm registry
129-
major_minor_patch_only = /\A\d+\.\d+\.\d+\z/
130-
react_on_rails_pkg = if ReactOnRails::VERSION.match?(major_minor_patch_only)
131-
"react-on-rails@#{ReactOnRails::VERSION}"
126+
# Use exact version match between gem and npm package for all versions including pre-releases
127+
# Ruby gem versions use dots (16.2.0.beta.10) but npm requires hyphens (16.2.0-beta.10)
128+
# This method converts between the two formats.
129+
#
130+
# The regex matches:
131+
# - Stable: 16.2.0
132+
# - Beta (Ruby): 16.2.0.beta.10 or (npm): 16.2.0-beta.10
133+
# - RC (Ruby): 16.1.0.rc.1 or (npm): 16.1.0-rc.1
134+
# - Alpha (Ruby): 16.0.0.alpha.5 or (npm): 16.0.0-alpha.5
135+
# This ensures beta/rc versions use the exact version instead of "latest" which would
136+
# install the latest stable release and cause version mismatches.
137+
138+
# Accept both dot and hyphen separators for pre-release versions
139+
version_with_optional_prerelease = /\A(\d+\.\d+\.\d+)([-.]([a-zA-Z0-9.]+))?\z/
140+
141+
react_on_rails_pkg = if (match = ReactOnRails::VERSION.match(version_with_optional_prerelease))
142+
base_version = match[1]
143+
prerelease = match[3]
144+
145+
# Convert Ruby gem format (dot) to npm semver format (hyphen)
146+
npm_version = if prerelease
147+
"#{base_version}-#{prerelease}"
148+
else
149+
base_version
150+
end
151+
152+
"react-on-rails@#{npm_version}"
132153
else
133-
puts "Adding the latest react-on-rails NPM module. " \
154+
puts "WARNING: Unrecognized version format #{ReactOnRails::VERSION}. " \
155+
"Adding the latest react-on-rails NPM module. " \
134156
"Double check this is correct in package.json"
135157
"react-on-rails"
136158
end

spec/react_on_rails/generators/js_dependency_manager_spec.rb

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,12 +208,57 @@ def errors
208208
expect(mock_manager).to have_received(:add).with(["[email protected]"], exact: true)
209209
end
210210

211-
it "adds react-on-rails without version for pre-releases" do
211+
it "adds react-on-rails with version for RC pre-releases (npm format with hyphen)" do
212212
stub_const("ReactOnRails::VERSION", "16.0.0-rc.1")
213213
instance.send(:add_react_on_rails_package)
214+
expect(mock_manager).to have_received(:add).with(["[email protected]"], exact: true)
215+
end
216+
217+
it "converts Ruby gem beta format to npm format" do
218+
stub_const("ReactOnRails::VERSION", "16.2.0.beta.10")
219+
instance.send(:add_react_on_rails_package)
220+
expect(mock_manager).to have_received(:add).with(["[email protected]"], exact: true)
221+
end
222+
223+
it "converts Ruby gem RC format to npm format" do
224+
stub_const("ReactOnRails::VERSION", "16.0.0.rc.1")
225+
instance.send(:add_react_on_rails_package)
226+
expect(mock_manager).to have_received(:add).with(["[email protected]"], exact: true)
227+
end
228+
229+
it "converts Ruby gem alpha format to npm format" do
230+
stub_const("ReactOnRails::VERSION", "16.0.0.alpha.5")
231+
instance.send(:add_react_on_rails_package)
232+
expect(mock_manager).to have_received(:add).with(["[email protected]"], exact: true)
233+
end
234+
235+
it "accepts npm format beta pre-releases (already with hyphen)" do
236+
stub_const("ReactOnRails::VERSION", "16.2.0-beta.10")
237+
instance.send(:add_react_on_rails_package)
238+
expect(mock_manager).to have_received(:add).with(["[email protected]"], exact: true)
239+
end
240+
241+
it "accepts npm format alpha releases (already with hyphen)" do
242+
stub_const("ReactOnRails::VERSION", "16.0.0-alpha.5")
243+
instance.send(:add_react_on_rails_package)
244+
expect(mock_manager).to have_received(:add).with(["[email protected]"], exact: true)
245+
end
246+
247+
it "adds react-on-rails without version for invalid version formats" do
248+
stub_const("ReactOnRails::VERSION", "invalid-version")
249+
instance.send(:add_react_on_rails_package)
214250
expect(mock_manager).to have_received(:add).with(["react-on-rails"], exact: true)
215251
end
216252

253+
it "warns about invalid version format when version doesn't match semver" do
254+
stub_const("ReactOnRails::VERSION", "invalid-version")
255+
256+
# Capture stdout to verify the warning message
257+
expect do
258+
instance.send(:add_react_on_rails_package)
259+
end.to output(/WARNING: Unrecognized version format invalid-version/).to_stdout
260+
end
261+
217262
it "adds warning when add_package fails" do
218263
allow(mock_manager).to receive(:add).and_return(false)
219264
instance.package_json = nil

0 commit comments

Comments
 (0)