Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump minimum yarn version suggested in package.json engines #9144

Closed
wants to merge 3 commits into from
Closed

Bump minimum yarn version suggested in package.json engines #9144

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e510be9
Bump minimum yarn version
NicholasEllul Apr 4, 2024
db30852
Update environment setup docs
NicholasEllul Apr 4, 2024
4721da3
Merge branch 'yarn-1.22.22' into ellul/bump-yarn-engines
NicholasEllul Apr 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/readme/environment.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ With the correct Node version installed, Yarn v1 can be installed sudo-less in y
npm install -g yarn
```

Ensure that the installed version is within the range of supported versions specified in the `engines` section of [package.json](https://github.com/MetaMask/metamask-mobile/blob/main/package.json).
Copy link
Contributor

@legobeat legobeat Apr 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we set the packageManager field in package.json, I think corepack enable should do the right thing for you. I have not tried it with yarn1 in this repo and made sure no other steps are required, though.

Copy link
Member

@leotm leotm Apr 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added the packageManager field here #9143 (comment)

and discovered #9143 (comment)

figured nvm install (v18.18.2) adds old [email protected] under-the-hood (npm list -g) which sadly only checks packageManager is a valid semver, but doesn't enforce it

i tried bumping corepack to latest (npm install -g corepack) which is [email protected] and now the yarn version is enforced

➜  metamask-mobile git:(yarn-1.22.22) ✗ yarn
! Corepack is about to download https://registry.yarnpkg.com/yarn/-/yarn-1.22.2222222.tgz
? Do you want to continue? [Y/n]

and without packageManager, latest corepack downloads and auto-adds "packageManager": "[email protected]+sha1.someLongHash" before falling to the package.json engine check - not good! so we deffo want the packageManager field

and didn't need to run corepack enable after upgrading

so we might want to mention too to run npm install -g corepack after nvm install (or nvm use if already there)

then something like

you may still need to run corepack enable

or could include corepack enable in our yarn setup script instead


## Android

Install [Android Studio](https://developer.android.com/studio)
Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,7 @@
},
"engines": {
"node": "~18.18.2",
"yarn": "^1.22.22"
"yarn": ">=1.22.22 <2.0.0"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is semantically equivalent to the previous version - is it actually more meaningful?

},
"lavamoat": {
"allowScripts": {
Expand Down
Loading