Fix install commands for different yarn versions #525
Conversation
The install command given for Yarn Classic only works for Yarn Berry. The command given for Yarn Berry works, but only due to backward compatibility.
WalkthroughThe changes in this pull request detail the upgrade from Shakapacker v7 to v8, highlighting the removal of deprecated functions, a shift to package manager-agnostic dependency management, and the discontinuation of support for Ruby 2.6 and Node v12. Key updates include the necessity of setting the Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
docs/v8_upgrade.md(2 hunks)
🔇 Additional comments (1)
docs/v8_upgrade.md (1)
24-24: LGTM! Valuable documentation about CDN gotchas.
This addition provides crucial information about the limitations of using empty SHAKAPACKER_ASSET_HOST with dynamic imports and static assets, along with clear workarounds.
| raise if File.exist?("package.json") && !(system "yarn install --frozen-lockfile") | ||
|
|
||
| # yarn v2+ (berry) | ||
| raise if File.exist?("package.json") && !(system "yarn install --frozen-lockfile") | ||
| raise if File.exist?("package.json") && !(system "yarn install --immutable") |
There was a problem hiding this comment.
Fix Yarn install commands documentation
The current documentation might be confusing as it suggests different flags for different Yarn versions. According to Yarn's documentation:
- For Yarn Classic (1.x):
--frozen-lockfileis the correct flag - For Yarn Berry (2+): Both
--immutableand--frozen-lockfilework (the latter for backward compatibility)
To make this clearer, consider this revision:
# yarn v1.x (classic)
- raise if File.exist?("package.json") && !(system "yarn install --frozen-lockfile")
+ # Use --frozen-lockfile for Yarn Classic
+ raise if File.exist?("package.json") && !(system "yarn install --frozen-lockfile")
# yarn v2+ (berry)
- raise if File.exist?("package.json") && !(system "yarn install --immutable")
+ # Both --immutable and --frozen-lockfile (for backward compatibility) work
+ raise if File.exist?("package.json") && !(system "yarn install --immutable")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| raise if File.exist?("package.json") && !(system "yarn install --frozen-lockfile") | |
| # yarn v2+ (berry) | |
| raise if File.exist?("package.json") && !(system "yarn install --frozen-lockfile") | |
| raise if File.exist?("package.json") && !(system "yarn install --immutable") | |
| # yarn v1.x (classic) | |
| # Use --frozen-lockfile for Yarn Classic | |
| raise if File.exist?("package.json") && !(system "yarn install --frozen-lockfile") | |
| # yarn v2+ (berry) | |
| # Both --immutable and --frozen-lockfile (for backward compatibility) work | |
| raise if File.exist?("package.json") && !(system "yarn install --immutable") |
Summary
The install command given for Yarn Classic only works for Yarn Berry. The command given for Yarn Berry works, but only due to backward compatibility. See https://yarnpkg.com/cli/install:
Separately, removed a duplicate sentence.
Pull Request checklist
[ ] Add/update test to cover these changes[ ] Update CHANGELOG fileOther Information
Summary by CodeRabbit
SHAKAPACKER_ASSET_HOSTenvironment variable.packageManagerproperty inpackage.json.check_yarnrake task tocheck_manager.