fix: install the latest major version of peer dependencies#576
Conversation
WalkthroughThe changelog was updated to reflect a fix related to installing the latest major version of peer dependencies. In the core logic, the method for extracting the major version from a peer dependency string was adjusted to handle multiple version alternatives separated by "||", now focusing on the last alternative. Changes
Sequence Diagram(s)sequenceDiagram
participant Installer
participant VersionString
Installer->>VersionString: Get peer dependency version string
VersionString-->>Installer: Return version string (could contain "||")
Installer->>Installer: Split version string by "||"
Installer->>Installer: Take last segment
Installer->>Installer: Extract major version number from last segment
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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/install/template.rb (1)
139-139: LGTM! The logic correctly implements selecting the latest major version.The change from
version.match(/(\d+)/)[1]toversion.split("||").last.match(/(\d+)/)[1]properly handles version strings with multiple alternatives (e.g., "^14.0.0 || ^16.0.0 || ^18.0.0") by selecting the last alternative before extracting the major version number.Consider adding error handling for edge cases:
- major_version = version.split("||").last.match(/(\d+)/)[1] + major_version_match = version.split("||").last.match(/(\d+)/) + raise "Invalid version format: #{version}" unless major_version_match + major_version = major_version_match[1]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
CHANGELOG.md(1 hunks)lib/install/template.rb(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
CHANGELOG.md
13-13: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
⏰ Context from checks skipped due to timeout of 90000ms (20)
- GitHub Check: Testing (ubuntu-latest, 3.0, gemfiles/Gemfile-rails.7.0.x)
- GitHub Check: Generator specs (ubuntu-latest, 3.4, gemfiles/Gemfile-rails.8.0.x)
- GitHub Check: Generator specs (ubuntu-latest, 3.4, gemfiles/Gemfile-rails.7.2.x)
- GitHub Check: Generator specs (ubuntu-latest, 3.3, gemfiles/Gemfile-rails.8.0.x)
- GitHub Check: Generator specs (ubuntu-latest, 3.3, gemfiles/Gemfile-rails.7.2.x)
- GitHub Check: Generator specs (ubuntu-latest, 3.4, gemfiles/Gemfile-rails.7.1.x)
- GitHub Check: Generator specs (ubuntu-latest, 3.2, gemfiles/Gemfile-rails.7.1.x)
- GitHub Check: Generator specs (ubuntu-latest, 3.1, gemfiles/Gemfile-rails.7.1.x)
- GitHub Check: Generator specs (ubuntu-latest, 3.2, gemfiles/Gemfile-rails.7.2.x)
- GitHub Check: Generator specs (ubuntu-latest, 3.1, gemfiles/Gemfile-rails.6.1.x)
- GitHub Check: Generator specs (ubuntu-latest, 3.1, gemfiles/Gemfile-rails.7.2.x)
- GitHub Check: Generator specs (ubuntu-latest, 3.2, gemfiles/Gemfile-rails.7.0.x)
- GitHub Check: Generator specs (ubuntu-latest, 3.0, gemfiles/Gemfile-rails.7.1.x)
- GitHub Check: Generator specs (ubuntu-latest, 3.1, gemfiles/Gemfile-rails.7.0.x)
- GitHub Check: Generator specs (ubuntu-latest, 3.0, gemfiles/Gemfile-rails.6.1.x)
- GitHub Check: Generator specs (ubuntu-latest, 3.0, gemfiles/Gemfile-rails.7.0.x)
- GitHub Check: Generator specs (ubuntu-latest, 2.7, gemfiles/Gemfile-rails.7.1.x)
- GitHub Check: Generator specs (ubuntu-latest, 2.7, gemfiles/Gemfile-rails.7.0.x)
- GitHub Check: Generator specs (ubuntu-latest, 3.0, gemfiles/Gemfile-rails.6.0.x)
- GitHub Check: Generator specs (ubuntu-latest, 2.7, gemfiles/Gemfile-rails.6.0.x)
🔇 Additional comments (1)
CHANGELOG.md (1)
13-13: LGTM! The changelog entry properly documents the fix.The entry clearly describes the change and includes appropriate attribution. The static analysis hint about list style can be ignored - the file consistently uses dashes for list items in this section.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
13-13: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
|
@justin808 @tomdracz could I get a review on this? |
2 participants
Summary
As our peer dependency version constraints are laid out from oldest version to latest version, we currently install the oldest version major of peer dependencies which can result in security vulnerabilities and just general lagging behind - while the older versions will likely be more compatible with older versions of Node, I think it's fine to assume a recent version of Node is being used when you're install Shakapacker on a project.
Pull Request checklist
Add/update test to cover these changesUpdate documentationOther Information
I don't think this needs to be considered breaking given it will only apply to new runs of Shakapacker install, which is a "do once per application" thing for consumers.
Summary by CodeRabbit
Documentation
Bug Fixes