Fix beta/RC version handling in generator #2066
Conversation
|
Warning Rate limit exceeded@justin808 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 1 minutes and 39 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
WalkthroughAdds prerelease-aware semver matching when pinning the react-on-rails npm package; introduces a new CI command alias (/run-skipped-tests) across docs and workflows; makes webpack-generated files list conditional (ReactOnRails Pro aware); extends tests for prerelease handling and updates related docs/workflows. Changes
Sequence Diagram(s)sequenceDiagram
participant Generator
participant Regex as VersionRegex
participant NPM as Installer
participant Logger
Generator->>Regex: receive requested react-on-rails version string
Regex-->>Generator: matches prerelease? (e.g., 16.2.0-beta.10)
alt matches prerelease
Generator->>NPM: install "react-on-rails@<exact-version>"
NPM-->>Generator: install success
else does not match
Generator->>Logger: warn about unrecognized/invalid version format
Generator->>NPM: install "react-on-rails" (latest)
NPM-->>Generator: install success
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Code Review - PR #2066: Fix beta/RC version handling in generatorSummaryThis PR fixes a critical bug where pre-release versions (beta, RC, alpha) were incorrectly handled by the generator, causing version mismatches between the gem and npm package. ✅ Strengths1. Problem Correctly IdentifiedThe root cause analysis is spot-on. The original regex 2. Solution is SoundThe new regex
The regex follows the Semantic Versioning 2.0.0 spec for pre-release identifiers. 3. Improved User Experience
4. Comments are ExcellentThe inline comments clearly explain:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
lib/generators/react_on_rails/js_dependency_manager.rb (1)
125-142: Regex update correctly pins prerelease versions; consider minor SemVer hardeningThe new
version_with_optional_prereleaseregex plusmatch?looks correct for your stated goal: it will pin both plainx.y.zand prerelease versions like16.2.0-beta.10, while still warning and falling back to"react-on-rails"for clearly invalid formats like16.2or16.2.0.1. That should resolve the gem/npm mismatch for beta/RC flows. ✅If you ever use richer SemVer forms (e.g., prerelease identifiers with hyphens like
beta-1, or build metadata like+build.42), the current pattern will not match them. As an optional improvement, you could slightly broaden the prerelease portion to align more closely with the SemVer spec:- version_with_optional_prerelease = /\A\d+\.\d+\.\d+(-[a-zA-Z0-9.]+)?\z/ + version_with_optional_prerelease = /\A\d+\.\d+\.\d+(-[0-9A-Za-z][0-9A-Za-z.\-]*)?\z/This still rejects
16.2,16.2.0.1, etc., but allows a wider range of valid prerelease identifiers if you need them in the future.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/generators/react_on_rails/js_dependency_manager.rb(1 hunks)
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: justin808
Repo: shakacode/react_on_rails PR: 1770
File: lib/generators/react_on_rails/templates/base/base/app/javascript/src/HelloWorld/ror_components/HelloWorld.client.jsx:2-2
Timestamp: 2025-09-16T08:01:11.146Z
Learning: React on Rails uses webpack CSS Modules configuration with namedExports: true, which requires the import syntax `import * as style from './file.module.css'` rather than the default export pattern. This configuration enables better tree shaking and bundle size optimization for CSS modules.
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: node_package/src/turbolinksUtils.ts:34-36
Timestamp: 2025-02-13T16:50:26.861Z
Learning: In React on Rails, when checking for Turbolinks version 5 using `turbolinksVersion5()`, always ensure `Turbolinks` exists first by checking `turbolinksInstalled()` to prevent TypeError when accessing properties.
Learnt from: alexeyr-ci2
Repo: shakacode/react_on_rails PR: 1732
File: spec/dummy/client/app-react16/startup/ReduxSharedStoreApp.client.jsx:40-44
Timestamp: 2025-04-26T21:55:55.874Z
Learning: In the react_on_rails project, files under `app-react16` directories are copied/moved to corresponding `/app` directories during the conversion process (removing the `-react16` suffix), which affects their relative import paths at runtime.
Learnt from: alexeyr-ci
Repo: shakacode/react_on_rails PR: 1687
File: spec/dummy/package.json:0-0
Timestamp: 2025-01-23T18:20:45.824Z
Learning: When adding or updating dependencies in spec/dummy/package.json, maintain version consistency with other package.json files in the codebase to avoid potential version conflicts.
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1875
File: lib/react_on_rails/utils.rb:112-124
Timestamp: 2025-10-23T17:22:01.074Z
Learning: In React on Rails, when Pro is installed but not licensed, the intended behavior is to raise an error on boot. The `react_on_rails_pro?` method validates licenses and should raise errors early (including during path resolution in methods like `server_bundle?`) to enforce licensing requirements rather than failing later with obscure errors.
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1781
File: node_package/src/ClientSideRenderer.ts:82-95
Timestamp: 2025-09-15T21:24:48.207Z
Learning: In React on Rails, the force_load feature includes both explicit `data-force-load="true"` usage and the ability to hydrate components during the page loading state (`document.readyState === 'loading'`). Both capabilities require a Pro license, so the condition `!railsContext.rorPro && (isComponentForceLoaded || document.readyState === 'loading')` correctly gates both scenarios.
Learnt from: alexeyr-ci
Repo: shakacode/react_on_rails PR: 1644
File: node_package/src/ReactOnRailsRSC.ts:87-87
Timestamp: 2024-12-12T13:07:09.929Z
Learning: When handling errors in 'node_package/src/ReactOnRailsRSC.ts', include the error stack in error messages in development and test environments to aid debugging.
📚 Learning: 2025-01-23T18:20:45.824Z
Learnt from: alexeyr-ci
Repo: shakacode/react_on_rails PR: 1687
File: spec/dummy/package.json:0-0
Timestamp: 2025-01-23T18:20:45.824Z
Learning: When adding or updating dependencies in spec/dummy/package.json, maintain version consistency with other package.json files in the codebase to avoid potential version conflicts.
Applied to files:
lib/generators/react_on_rails/js_dependency_manager.rb
📚 Learning: 2025-02-12T16:38:06.537Z
Learnt from: Romex91
Repo: shakacode/react_on_rails PR: 1697
File: package-scripts.yml:28-28
Timestamp: 2025-02-12T16:38:06.537Z
Learning: The file `node_package/lib/ReactOnRails.full.js` is autogenerated during the build process and should not be present in the repository.
Applied to files:
lib/generators/react_on_rails/js_dependency_manager.rb
📚 Learning: 2025-04-26T21:55:55.874Z
Learnt from: alexeyr-ci2
Repo: shakacode/react_on_rails PR: 1732
File: spec/dummy/client/app-react16/startup/ReduxSharedStoreApp.client.jsx:40-44
Timestamp: 2025-04-26T21:55:55.874Z
Learning: In the react_on_rails project, files under `app-react16` directories are copied/moved to corresponding `/app` directories during the conversion process (removing the `-react16` suffix), which affects their relative import paths at runtime.
Applied to files:
lib/generators/react_on_rails/js_dependency_manager.rb
📚 Learning: 2025-09-16T08:01:11.146Z
Learnt from: justin808
Repo: shakacode/react_on_rails PR: 1770
File: lib/generators/react_on_rails/templates/base/base/app/javascript/src/HelloWorld/ror_components/HelloWorld.client.jsx:2-2
Timestamp: 2025-09-16T08:01:11.146Z
Learning: React on Rails uses webpack CSS Modules configuration with namedExports: true, which requires the import syntax `import * as style from './file.module.css'` rather than the default export pattern. This configuration enables better tree shaking and bundle size optimization for CSS modules.
Applied to files:
lib/generators/react_on_rails/js_dependency_manager.rb
📚 Learning: 2025-10-23T17:22:01.074Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1875
File: lib/react_on_rails/utils.rb:112-124
Timestamp: 2025-10-23T17:22:01.074Z
Learning: In React on Rails, when Pro is installed but not licensed, the intended behavior is to raise an error on boot. The `react_on_rails_pro?` method validates licenses and should raise errors early (including during path resolution in methods like `server_bundle?`) to enforce licensing requirements rather than failing later with obscure errors.
Applied to files:
lib/generators/react_on_rails/js_dependency_manager.rb
📚 Learning: 2025-02-13T16:50:26.861Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: node_package/src/turbolinksUtils.ts:34-36
Timestamp: 2025-02-13T16:50:26.861Z
Learning: In React on Rails, when checking for Turbolinks version 5 using `turbolinksVersion5()`, always ensure `Turbolinks` exists first by checking `turbolinksInstalled()` to prevent TypeError when accessing properties.
Applied to files:
lib/generators/react_on_rails/js_dependency_manager.rb
📚 Learning: 2025-02-18T13:08:01.477Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: lib/react_on_rails/helper.rb:190-197
Timestamp: 2025-02-18T13:08:01.477Z
Learning: RSC support validation in React on Rails Pro is handled through a chain of validations:
1. Pro version check in `run_stream_inside_fiber`
2. RSC support check during pack generation via `ReactOnRailsPro.configuration.enable_rsc_support`
3. RSC support validation during component registration
This makes additional validation in the helper methods unnecessary.
Applied to files:
lib/generators/react_on_rails/js_dependency_manager.rb
📚 Learning: 2025-02-13T16:50:47.848Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: node_package/src/clientStartup.ts:18-21
Timestamp: 2025-02-13T16:50:47.848Z
Learning: In the react_on_rails module, the `reactOnRailsPageUnloaded` function in clientStartup.ts is intentionally kept private as it's only used internally as a callback for `onPageUnloaded`.
Applied to files:
lib/generators/react_on_rails/js_dependency_manager.rb
📚 Learning: 2025-09-15T21:24:48.207Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1781
File: node_package/src/ClientSideRenderer.ts:82-95
Timestamp: 2025-09-15T21:24:48.207Z
Learning: In React on Rails, the force_load feature includes both explicit `data-force-load="true"` usage and the ability to hydrate components during the page loading state (`document.readyState === 'loading'`). Both capabilities require a Pro license, so the condition `!railsContext.rorPro && (isComponentForceLoaded || document.readyState === 'loading')` correctly gates both scenarios.
Applied to files:
lib/generators/react_on_rails/js_dependency_manager.rb
📚 Learning: 2024-12-12T13:07:09.929Z
Learnt from: alexeyr-ci
Repo: shakacode/react_on_rails PR: 1644
File: node_package/src/ReactOnRailsRSC.ts:87-87
Timestamp: 2024-12-12T13:07:09.929Z
Learning: When handling errors in 'node_package/src/ReactOnRailsRSC.ts', include the error stack in error messages in development and test environments to aid debugging.
Applied to files:
lib/generators/react_on_rails/js_dependency_manager.rb
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build-dummy-app-webpack-test-bundles (3.4, 22, latest)
- GitHub Check: rspec-package-tests (3.4, latest)
- GitHub Check: build
- GitHub Check: claude-review
Code Review: Fix beta/RC version handling in generator✅ Overall AssessmentThis is an excellent fix that addresses a critical bug in version handling for pre-release versions. The implementation is well-thought-out, thoroughly tested, and properly documented. 🎯 Problem AnalysisRoot Cause Identified Correctly: The regex Impact: During beta/RC releases, users would get version mismatches between the gem and npm package, breaking the installation. ✅ Code Quality1. Regex Pattern ( version_with_optional_prerelease = /\A\d+\.\d+\.\d+(-[a-zA-Z0-9.]+)?\z/✅ Correct and comprehensive:
✅ Edge cases covered:
2. Documentation ( ✅ Excellent inline comments:
3. Error Messaging ( ✅ Improved warning:
✅ Test CoverageComprehensive test additions ( ✅ All pre-release formats tested:
✅ Test quality:
🔍 Potential Concerns & Recommendations1. Consistency with VersionChecker ✅The new regex is consistent with 2. Build Metadata Not Supported
|
|
/run-skipped-tests |
|
/run-skipped-ci |
|
🚀 Full CI Mode Enabled Failed to trigger:
Note: Added the To disable full CI mode, use the View progress in the Actions tab. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
CONTRIBUTING.md (1)
429-437: Add a language to the fenced CI command block for markdownlint complianceThe new
/run-skipped-ci//run-skipped-testsexample block is clear, but markdownlint (MD040) prefers a language on fenced code blocks. You can fix this by tagging it as shell commands:-``` +```bash /run-skipped-ci # or use the shorter alias: /run-skipped-tests</blockquote></details> <details> <summary>.github/read-me.md (1)</summary><blockquote> `7-15`: **Tag the CI command example fence with a language** The `/run-skipped-ci` / `/run-skipped-tests` example is good; to satisfy markdownlint (MD040) and improve rendering, consider specifying a language: ```diff -``` +```bash /run-skipped-ci # or use the shorter alias: /run-skipped-tests</blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: CodeRabbit UI **Review profile**: CHILL **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 81e717e96063cd68259d6ca4aad5971f9ceda3a8 and 4304d815be4b5335cc8d3a17f951162e8bb055c7. </details> <details> <summary>📒 Files selected for processing (6)</summary> * `.github/read-me.md` (5 hunks) * `.github/workflows/detect-invalid-ci-commands.yml` (4 hunks) * `.github/workflows/run-skipped-ci.yml` (2 hunks) * `CONTRIBUTING.md` (1 hunks) * `lib/react_on_rails/configuration.rb` (2 hunks) * `spec/react_on_rails/generators/js_dependency_manager_spec.rb` (1 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🧠 Learnings (14)</summary> <details> <summary>📓 Common learnings</summary>Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1875
File: lib/react_on_rails/utils.rb:112-124
Timestamp: 2025-10-23T17:22:01.074Z
Learning: In React on Rails, when Pro is installed but not licensed, the intended behavior is to raise an error on boot. Thereact_on_rails_pro?method validates licenses and should raise errors early (including during path resolution in methods likeserver_bundle?) to enforce licensing requirements rather than failing later with obscure errors.Learnt from: justin808
Repo: shakacode/react_on_rails PR: 1770
File: lib/generators/react_on_rails/templates/base/base/app/javascript/src/HelloWorld/ror_components/HelloWorld.client.jsx:2-2
Timestamp: 2025-09-16T08:01:11.146Z
Learning: React on Rails uses webpack CSS Modules configuration with namedExports: true, which requires the import syntaximport * as style from './file.module.css'rather than the default export pattern. This configuration enables better tree shaking and bundle size optimization for CSS modules.Learnt from: alexeyr-ci
Repo: shakacode/react_on_rails PR: 1644
File: node_package/src/ReactOnRailsRSC.ts:87-87
Timestamp: 2024-12-12T13:07:09.929Z
Learning: When handling errors in 'node_package/src/ReactOnRailsRSC.ts', include the error stack in error messages in development and test environments to aid debugging.Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: lib/react_on_rails/helper.rb:190-197
Timestamp: 2025-02-18T13:08:01.477Z
Learning: RSC support validation in React on Rails Pro is handled through a chain of validations:
- Pro version check in
run_stream_inside_fiber- RSC support check during pack generation via
ReactOnRailsPro.configuration.enable_rsc_support- RSC support validation during component registration
This makes additional validation in the helper methods unnecessary.Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1781
File: node_package/src/ClientSideRenderer.ts:82-95
Timestamp: 2025-09-15T21:24:48.207Z
Learning: In React on Rails, the force_load feature includes both explicitdata-force-load="true"usage and the ability to hydrate components during the page loading state (document.readyState === 'loading'). Both capabilities require a Pro license, so the condition!railsContext.rorPro && (isComponentForceLoaded || document.readyState === 'loading')correctly gates both scenarios.Learnt from: alexeyr-ci2
Repo: shakacode/react_on_rails PR: 1732
File: spec/dummy/client/app-react16/startup/ReduxSharedStoreApp.client.jsx:40-44
Timestamp: 2025-04-26T21:55:55.874Z
Learning: In the react_on_rails project, files underapp-react16directories are copied/moved to corresponding/appdirectories during the conversion process (removing the-react16suffix), which affects their relative import paths at runtime.Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: node_package/src/turbolinksUtils.ts:34-36
Timestamp: 2025-02-13T16:50:26.861Z
Learning: In React on Rails, when checking for Turbolinks version 5 usingturbolinksVersion5(), always ensureTurbolinksexists first by checkingturbolinksInstalled()to prevent TypeError when accessing properties.Learnt from: alexeyr-ci
Repo: shakacode/react_on_rails PR: 1687
File: spec/dummy/package.json:0-0
Timestamp: 2025-01-23T18:20:45.824Z
Learning: When adding or updating dependencies in spec/dummy/package.json, maintain version consistency with other package.json files in the codebase to avoid potential version conflicts.</details> <details> <summary>📚 Learning: 2025-01-23T18:20:45.824Z</summary>Learnt from: alexeyr-ci
Repo: shakacode/react_on_rails PR: 1687
File: spec/dummy/package.json:0-0
Timestamp: 2025-01-23T18:20:45.824Z
Learning: When adding or updating dependencies in spec/dummy/package.json, maintain version consistency with other package.json files in the codebase to avoid potential version conflicts.**Applied to files:** - `spec/react_on_rails/generators/js_dependency_manager_spec.rb` </details> <details> <summary>📚 Learning: 2025-02-12T16:38:06.537Z</summary>Learnt from: Romex91
Repo: shakacode/react_on_rails PR: 1697
File: package-scripts.yml:28-28
Timestamp: 2025-02-12T16:38:06.537Z
Learning: The filenode_package/lib/ReactOnRails.full.jsis autogenerated during the build process and should not be present in the repository.**Applied to files:** - `spec/react_on_rails/generators/js_dependency_manager_spec.rb` - `lib/react_on_rails/configuration.rb` </details> <details> <summary>📚 Learning: 2025-04-26T21:55:55.874Z</summary>Learnt from: alexeyr-ci2
Repo: shakacode/react_on_rails PR: 1732
File: spec/dummy/client/app-react16/startup/ReduxSharedStoreApp.client.jsx:40-44
Timestamp: 2025-04-26T21:55:55.874Z
Learning: In the react_on_rails project, files underapp-react16directories are copied/moved to corresponding/appdirectories during the conversion process (removing the-react16suffix), which affects their relative import paths at runtime.**Applied to files:** - `spec/react_on_rails/generators/js_dependency_manager_spec.rb` - `lib/react_on_rails/configuration.rb` </details> <details> <summary>📚 Learning: 2024-12-12T13:07:09.929Z</summary>Learnt from: alexeyr-ci
Repo: shakacode/react_on_rails PR: 1644
File: node_package/src/ReactOnRailsRSC.ts:87-87
Timestamp: 2024-12-12T13:07:09.929Z
Learning: When handling errors in 'node_package/src/ReactOnRailsRSC.ts', include the error stack in error messages in development and test environments to aid debugging.**Applied to files:** - `spec/react_on_rails/generators/js_dependency_manager_spec.rb` </details> <details> <summary>📚 Learning: 2025-09-16T08:01:11.146Z</summary>Learnt from: justin808
Repo: shakacode/react_on_rails PR: 1770
File: lib/generators/react_on_rails/templates/base/base/app/javascript/src/HelloWorld/ror_components/HelloWorld.client.jsx:2-2
Timestamp: 2025-09-16T08:01:11.146Z
Learning: React on Rails uses webpack CSS Modules configuration with namedExports: true, which requires the import syntaximport * as style from './file.module.css'rather than the default export pattern. This configuration enables better tree shaking and bundle size optimization for CSS modules.**Applied to files:** - `spec/react_on_rails/generators/js_dependency_manager_spec.rb` - `lib/react_on_rails/configuration.rb` </details> <details> <summary>📚 Learning: 2025-10-23T17:22:01.074Z</summary>Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1875
File: lib/react_on_rails/utils.rb:112-124
Timestamp: 2025-10-23T17:22:01.074Z
Learning: In React on Rails, when Pro is installed but not licensed, the intended behavior is to raise an error on boot. Thereact_on_rails_pro?method validates licenses and should raise errors early (including during path resolution in methods likeserver_bundle?) to enforce licensing requirements rather than failing later with obscure errors.**Applied to files:** - `spec/react_on_rails/generators/js_dependency_manager_spec.rb` - `lib/react_on_rails/configuration.rb` </details> <details> <summary>📚 Learning: 2025-02-18T13:08:01.477Z</summary>Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: lib/react_on_rails/helper.rb:190-197
Timestamp: 2025-02-18T13:08:01.477Z
Learning: RSC support validation in React on Rails Pro is handled through a chain of validations:
- Pro version check in
run_stream_inside_fiber- RSC support check during pack generation via
ReactOnRailsPro.configuration.enable_rsc_support- RSC support validation during component registration
This makes additional validation in the helper methods unnecessary.**Applied to files:** - `spec/react_on_rails/generators/js_dependency_manager_spec.rb` - `lib/react_on_rails/configuration.rb` </details> <details> <summary>📚 Learning: 2025-07-08T05:57:29.630Z</summary>Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1745
File: node_package/src/RSCRequestTracker.ts:8-14
Timestamp: 2025-07-08T05:57:29.630Z
Learning: The globalgenerateRSCPayloadfunction in React on Rails Pro (RORP) is provided by the framework during rendering requests, not implemented in application code. Thedeclare globalstatements are used to document the expected interface that RORP will inject at runtime.**Applied to files:** - `lib/react_on_rails/configuration.rb` </details> <details> <summary>📚 Learning: 2025-09-15T21:24:48.207Z</summary>Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1781
File: node_package/src/ClientSideRenderer.ts:82-95
Timestamp: 2025-09-15T21:24:48.207Z
Learning: In React on Rails, the force_load feature includes both explicitdata-force-load="true"usage and the ability to hydrate components during the page loading state (document.readyState === 'loading'). Both capabilities require a Pro license, so the condition!railsContext.rorPro && (isComponentForceLoaded || document.readyState === 'loading')correctly gates both scenarios.**Applied to files:** - `lib/react_on_rails/configuration.rb` </details> <details> <summary>📚 Learning: 2025-02-13T16:50:26.861Z</summary>Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: node_package/src/turbolinksUtils.ts:34-36
Timestamp: 2025-02-13T16:50:26.861Z
Learning: In React on Rails, when checking for Turbolinks version 5 usingturbolinksVersion5(), always ensureTurbolinksexists first by checkingturbolinksInstalled()to prevent TypeError when accessing properties.**Applied to files:** - `lib/react_on_rails/configuration.rb` </details> <details> <summary>📚 Learning: 2025-02-13T16:50:47.848Z</summary>Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: node_package/src/clientStartup.ts:18-21
Timestamp: 2025-02-13T16:50:47.848Z
Learning: In the react_on_rails module, thereactOnRailsPageUnloadedfunction in clientStartup.ts is intentionally kept private as it's only used internally as a callback foronPageUnloaded.**Applied to files:** - `lib/react_on_rails/configuration.rb` </details> <details> <summary>📚 Learning: 2025-04-09T12:56:10.756Z</summary>Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1696
File: node_package/src/RSCPayloadContainer.ts:0-0
Timestamp: 2025-04-09T12:56:10.756Z
Learning: In the react_on_rails codebase, RSC payloads are already stringified usingJSON.stringify()before being processed by theescapeScriptfunction, which handles escaping of special characters. The function only needs to handle specific HTML markers like comments and closing script tags.**Applied to files:** - `lib/react_on_rails/configuration.rb` </details> <details> <summary>📚 Learning: 2025-02-18T13:08:01.477Z</summary>Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: lib/react_on_rails/helper.rb:190-197
Timestamp: 2025-02-18T13:08:01.477Z
Learning: RSC support validation is handled in deeper level calls of the React on Rails Pro codebase, so it doesn't need to be validated again in thersc_payload_react_componenthelper method.**Applied to files:** - `lib/react_on_rails/configuration.rb` </details> </details><details> <summary>🧬 Code graph analysis (1)</summary> <details> <summary>lib/react_on_rails/configuration.rb (2)</summary><blockquote> <details> <summary>lib/react_on_rails/utils.rb (1)</summary> * `react_on_rails_pro?` (256-264) </details> <details> <summary>react_on_rails_pro/lib/react_on_rails_pro/configuration.rb (1)</summary> * `configuration` (9-38) </details> </blockquote></details> </details><details> <summary>🪛 markdownlint-cli2 (0.18.1)</summary> <details> <summary>.github/read-me.md</summary> 11-11: Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> <details> <summary>CONTRIBUTING.md</summary> 433-433: Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> </details> <details> <summary>⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)</summary> * GitHub Check: examples (3.4, latest) * GitHub Check: examples (3.2, minimum) * GitHub Check: rspec-package-tests (3.2, minimum) * GitHub Check: rspec-package-tests (3.4, latest) * GitHub Check: build-dummy-app-webpack-test-bundles (3.4, 22, latest) * GitHub Check: build-dummy-app-webpack-test-bundles * GitHub Check: build-dummy-app-webpack-test-bundles * GitHub Check: build * GitHub Check: pro-lint-js-and-ruby </details> <details> <summary>🔇 Additional comments (6)</summary><blockquote> <details> <summary>lib/react_on_rails/configuration.rb (2)</summary><blockquote> `3-19`: **ActiveSupport requires and `compact_blank` polyfill look safe and compatible** Requiring the enumerable and blank? extensions and guarding the `compact_blank` polyfill with `[].respond_to?(:compact_blank)` avoids redefining methods on newer Rails while providing the expected `reject(&:blank?)` semantics on older versions. This global patch is reasonable for a Rails-centric gem. --- `431-444`: **Dynamic default `webpack_generated_files` for Pro and non‑Pro setups looks correct** The new `ensure_webpack_generated_files_exists` logic cleanly builds a default list from `manifest.json`, `server_bundle_js_file`, and (when licensed Pro is available) the RSC and manifest paths from `ReactOnRailsPro.configuration`, then filters with `compact_blank`. Using `ReactOnRails::Utils.react_on_rails_pro?` here also enforces Pro licensing early, which matches the intended behavior from prior work. Based on learnings. </blockquote></details> <details> <summary>spec/react_on_rails/generators/js_dependency_manager_spec.rb (1)</summary><blockquote> `211-243`: **Pre‑release and invalid‑format coverage for `add_react_on_rails_package` is solid** The new examples for rc, beta, alpha, and invalid versions exercise the intended semver‑aware behavior (pinning for valid pre‑releases, falling back to unversioned install + warning for invalid formats) and keep expectations on `mock_manager.add` and STDOUT nicely focused per scenario. This aligns well with the generator change. </blockquote></details> <details> <summary>.github/workflows/detect-invalid-ci-commands.yml (1)</summary><blockquote> `20-23`: **Alias handling for `/run-skipped-tests` is wired consistently** Updating the job guard, `validCommands` list, and help text to include `/run-skipped-tests` cleanly treats it as an alias for `/run-skipped-ci` while keeping `/stop-run-skipped-ci` behavior unchanged. This avoids false “invalid command” hints for the new alias and keeps user guidance in sync. Also applies to: 46-52, 82-99 </blockquote></details> <details> <summary>.github/workflows/run-skipped-ci.yml (2)</summary><blockquote> `18-21`: **Command alias logic is correct.** The trigger condition properly extends to recognize `/run-skipped-tests` alongside `/run-skipped-ci`, handling both start-of-comment and mid-comment placements. The YAML conditional syntax is sound. Verify that the `/run-skipped-tests` command alias is documented in `README.md`, `CONTRIBUTING.md`, and/or relevant workflow documentation so users are aware of both command options. --- `119-123`: **Disregard this review comment—the file was not modified in this PR.** Git verification shows `.github/workflows/run-skipped-ci.yml` has no changes between the main branch and the current HEAD. The original review comment was based on an incorrect AI summary claiming a 'Main test' removal occurred, but no modifications to this file exist in this pull request. All workflow files referenced in the existing workflowMap are confirmed to exist and the code is functioning as intended. > Likely an incorrect or invalid review comment. </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
The regex for detecting valid npm package versions only matched stable releases (e.g., 16.2.0) but not pre-release versions (e.g., 16.2.0-beta.10). This caused the generator to install 'react-on-rails' (latest stable) instead of '[email protected]', leading to version mismatch errors when running the generator with beta versions. Changes: - Updated regex from /\A\d+\.\d+\.\d+\z/ to /\A\d+\.\d+\.\d+(-[a-zA-Z0-9.]+)?\z/ - Now matches stable (16.2.0), beta (16.2.0-beta.10), RC (16.1.0-rc.1), etc. - Updated comments to explain the regex and why exact versions are needed - Improved warning message when version format is unrecognized This fixes the version mismatch error seen in CI: Package: 16.1.2 (latest stable from npm) Gem: 16.2.0.beta.10 (current beta version) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Updated the test expectations to match the new behavior where pre-release versions (beta, RC, alpha) are installed with exact version specifiers instead of installing the latest stable version. Changes: - Updated test name from 'without version' to 'with version' for pre-releases - Updated expectation to include version: [email protected] - Added test for beta versions: [email protected] - Added test for alpha versions: [email protected] - Added test for invalid version formats with warning message check All 39 examples now pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Ruby gem versions use dots for pre-release versions (e.g., 16.2.0.beta.10) while npm requires hyphens (16.2.0-beta.10) to match semver spec. This commit fixes the generator to convert between the two formats so that beta/RC/alpha versions are installed correctly. Before this fix: - Generator would try to install [email protected] (invalid npm version) - This would fail to match any published version - yalc link would work, but package.json would have wrong version - Version checker would then fail due to version mismatch After this fix: - Generator converts 16.2.0.beta.10 → 16.2.0-beta.10 - npm installs the correct version - Version checker validates successfully The fix also handles both formats as input (dot or hyphen) and normalizes to the correct npm format. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
This file was added to master without proper prettier formatting. Running prettier --write to fix list spacing issues. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
justin808
force-pushed
the
justin808/fix-beta-yalc-version
branch
from
1cee1e5 to
a68a7cc
Compare
Code Review: Fix beta/RC version handling in generatorThank you for tackling this important bug! The fix correctly addresses the version mismatch issue for pre-release versions. Here's my detailed review: ✅ What's Working Well
|
…se-otp-timing * origin/master: (27 commits) Fix doctor command false version mismatch for beta/prerelease versions (#2064) Fix beta/RC version handling in generator (#2066) Document Rails Engine development nuances and add tests for automatic rake task loading (#2067) Add /run-skipped-tests as alias for /run-skipped-ci (#XXXX) (#2068) Fix: Add Rails 5.2-6.0 compatibility for compact_blank (#2058) Break CI circular dependency with non-docs change (#2065) Fix CI safety check to evaluate latest workflow attempt (#2062) Fix yalc publish (#2054) Add Shakapacker 9.0+ private_output_path integration for server bundles (#2028) Consolidate all beta versions into v16.2.0.beta.10 (#2057) Improve reliability of CI debugging scripts (#2056) Clarify monorepo changelog structure in documentation (#2055) Bump version to 16.2.0.beta.10 Bump version to 16.2.0.beta.9 Fix duplicate rake task execution by removing explicit task loading (#2052) Simplify precompile hook and restore Pro dummy app to async loading (#2053) Add Shakapacker precompile hook with ReScript support to Pro dummy app (#1977) Guard master docs-only pushes and ensure full CI runs (#2042) Refactor: Extract JS dependency management into shared module (#2051) Add workflow to detect invalid CI command attempts (#2037) ... # Conflicts: # rakelib/release.rake
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 like16.2.0. This caused the generator to installreact-on-rails(latest stable from npm) instead of[email protected], leading to version mismatch errors:Root Cause
In
lib/generators/react_on_rails/js_dependency_manager.rb, the regex was: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:
This now matches:
16.2.016.2.0-beta.1016.1.0-rc.116.0.0-alpha.51.2.3-pre.release.1Changes
(-[a-zA-Z0-9.]+)?Testing
Verified the regex matches all expected version formats:
16.2.0,16.1.216.2.0-beta.1016.1.0-rc.116.0.0-alpha.5invalid,16.2,16.2.0.1Impact
This fix ensures that:
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
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.