Fixes the lockfile hydration of the npm: protocol
#9023
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Ran the tests locally since they don't run on CI anymore; no tests started to fail between 1.22-stable and this PR, so it should be fine to merge. Trying a patch release now. |
13 tasks
tommasini
added a commit
to MetaMask/metamask-mobile
that referenced
this pull request
Mar 13, 2024
## **Description** Yarn version was updated to 1.22.22 and we were impacted, on this PR we update yarn version on bitrise to be the same as on the github action for consistency and we solve duplicated issues on our lockfile yarn was updated to fix a bug, more details [here](yarnpkg/yarn#9023) ## **Related issues** Fixes: ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've clearly explained what problem this PR is solving and how it is solved. - [ ] I've linked related issues - [ ] I've included manual testing steps - [ ] I've included screenshots/recordings if applicable - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. - [ ] I’ve properly set the pull request status: - [ ] In case it's not yet "ready for review", I've set it to "draft". - [ ] In case it's "ready for review", I've changed it from "draft" to "non-draft". ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
leotm
added a commit
to MetaMask/metamask-mobile
that referenced
this pull request
Apr 4, 2024
Docs: https://classic.yarnpkg.com/lang/en/docs/package-json/#toc-engines Contains lockfile hydration fix - PR: yarnpkg/yarn#9023 - Commit: yarnpkg/yarn@88d5e44
7 tasks
16 tasks
leotm
added a commit
to MetaMask/metamask-mobile
that referenced
this pull request
Apr 30, 2024
## **Description** Prevent engineers using different Yarn versions with different features causing our CI dedupe step to break - [x] Bump package.json Yarn engine from ^1.22.0 to 1.22.22 - [x] Bump BitRise Yarn version from 1.22.19 to 1.22.22 - [x] Lock project Yarn version to 1.22.22 - [x] Add Yarn config file comment RE @lavamoat/allow-scripts setup - [x] Run `yarn deduplicate` and commit - [x] Revert Bump BitRise Yarn version from 1.22.19 to 1.22.22 ([thread](#9143 (review))) - [x] Bump BitRise Yarn version from 1.22.19 to 1.22.22 - [x] Fix yarnpkg/yarn#9050 - [x] Switch to <s>NPM or</s> Corepack Nb: our metamask-extension setup - https://github.com/MetaMask/metamask-extension/blob/develop/package.json#L616 - https://github.com/MetaMask/metamask-extension/blob/develop/package.json#L681 - https://github.com/MetaMask/metamask-extension/blob/develop/.yarn/releases/yarn-4.0.2.cjs - https://github.com/MetaMask/metamask-extension/blob/develop/.yarnrc.yml#L142 https://classic.yarnpkg.com/lang/en/docs/package-json/#toc-engines > The engines specify versions of clients that must be used with your package. This checks against `process.versions` as well as the current version of yarn. https://classic.yarnpkg.com/lang/en/docs/yarnrc/#toc-yarn-path > Instructs yarn to defer to another Yarn binary for execution. Useful if you want to bundle Yarn into your repository and have everyone use the same version for consistency. This was introduced in Yarn 1.0, so all developers must have Yarn >= 1.0 installed. https://yarnpkg.com/getting-started/qa > `.yarn/plugins` and `.yarn/releases` contain the Yarn releases used in the current repository (as defined by [yarn ](https://yarnpkg.com/cli/set/version)[set version](https://yarnpkg.com/cli/set/version)). You will want to keep them versioned (this prevents potential issues if, say, two engineers use different Yarn versions with different features). https://yarnpkg.com/cli/set/version > Lock the Yarn version used by the project. https://nodejs.org/dist/latest-v15.x/docs/api/all.html (introduced in Node v16 only) https://nodejs.org/dist/latest-v16.x/docs/api/all.html#all_packages_packagemanager from testing our repo (on Node v18), this appears to have no effect enforcing the version from testing our repo, we could revert c7d67e6 to keep our structure simpler since the package.json engine field is enough to enforce the version but these 2 files are generated by running `yarn set version 1.22.22` Yarn v1.22 update - PR: yarnpkg/yarn#9023 - Commit: yarnpkg/yarn@88d5e44 - Picked into branch: https://github.com/yarnpkg/yarn/tree/1.22-stable ## **Related issues** Fixes: CI dedupe step consistently failing ## **Manual testing steps** - ensure `yarn --version` is below 1.22.22 - or e.g. `yarn set version 1.22.21` - run `yarn` - Yarn errors correctly ^1.22.22+ is expected Nb: after doing this too many times, you can use a VPN as a workaround <img width="1806" alt="Screenshot 2024-04-04 at 6 40 35 PM" src="https://github.com/MetaMask/metamask-mobile/assets/1881059/41cf015c-2aed-4c63-8f24-041a5cf98ef3"> ## **Screenshots/Recordings** <img width="987" alt="Screenshot 2024-04-04 at 8 32 00 PM" src="https://github.com/MetaMask/metamask-mobile/assets/1881059/901ca091-02db-4a69-8137-0ec1b6b3c07d"> ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [x] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --------- Co-authored-by: legobeat <109787230+legobeat@users.noreply.github.com>
abretonc7s
pushed a commit
to MetaMask/metamask-mobile
that referenced
this pull request
May 9, 2024
## **Description** Prevent engineers using different Yarn versions with different features causing our CI dedupe step to break - [x] Bump package.json Yarn engine from ^1.22.0 to 1.22.22 - [x] Bump BitRise Yarn version from 1.22.19 to 1.22.22 - [x] Lock project Yarn version to 1.22.22 - [x] Add Yarn config file comment RE @lavamoat/allow-scripts setup - [x] Run `yarn deduplicate` and commit - [x] Revert Bump BitRise Yarn version from 1.22.19 to 1.22.22 ([thread](#9143 (review))) - [x] Bump BitRise Yarn version from 1.22.19 to 1.22.22 - [x] Fix yarnpkg/yarn#9050 - [x] Switch to <s>NPM or</s> Corepack Nb: our metamask-extension setup - https://github.com/MetaMask/metamask-extension/blob/develop/package.json#L616 - https://github.com/MetaMask/metamask-extension/blob/develop/package.json#L681 - https://github.com/MetaMask/metamask-extension/blob/develop/.yarn/releases/yarn-4.0.2.cjs - https://github.com/MetaMask/metamask-extension/blob/develop/.yarnrc.yml#L142 https://classic.yarnpkg.com/lang/en/docs/package-json/#toc-engines > The engines specify versions of clients that must be used with your package. This checks against `process.versions` as well as the current version of yarn. https://classic.yarnpkg.com/lang/en/docs/yarnrc/#toc-yarn-path > Instructs yarn to defer to another Yarn binary for execution. Useful if you want to bundle Yarn into your repository and have everyone use the same version for consistency. This was introduced in Yarn 1.0, so all developers must have Yarn >= 1.0 installed. https://yarnpkg.com/getting-started/qa > `.yarn/plugins` and `.yarn/releases` contain the Yarn releases used in the current repository (as defined by [yarn ](https://yarnpkg.com/cli/set/version)[set version](https://yarnpkg.com/cli/set/version)). You will want to keep them versioned (this prevents potential issues if, say, two engineers use different Yarn versions with different features). https://yarnpkg.com/cli/set/version > Lock the Yarn version used by the project. https://nodejs.org/dist/latest-v15.x/docs/api/all.html (introduced in Node v16 only) https://nodejs.org/dist/latest-v16.x/docs/api/all.html#all_packages_packagemanager from testing our repo (on Node v18), this appears to have no effect enforcing the version from testing our repo, we could revert c7d67e6 to keep our structure simpler since the package.json engine field is enough to enforce the version but these 2 files are generated by running `yarn set version 1.22.22` Yarn v1.22 update - PR: yarnpkg/yarn#9023 - Commit: yarnpkg/yarn@88d5e44 - Picked into branch: https://github.com/yarnpkg/yarn/tree/1.22-stable ## **Related issues** Fixes: CI dedupe step consistently failing ## **Manual testing steps** - ensure `yarn --version` is below 1.22.22 - or e.g. `yarn set version 1.22.21` - run `yarn` - Yarn errors correctly ^1.22.22+ is expected Nb: after doing this too many times, you can use a VPN as a workaround <img width="1806" alt="Screenshot 2024-04-04 at 6 40 35 PM" src="https://github.com/MetaMask/metamask-mobile/assets/1881059/41cf015c-2aed-4c63-8f24-041a5cf98ef3"> ## **Screenshots/Recordings** <img width="987" alt="Screenshot 2024-04-04 at 8 32 00 PM" src="https://github.com/MetaMask/metamask-mobile/assets/1881059/901ca091-02db-4a69-8137-0ec1b6b3c07d"> ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [x] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --------- Co-authored-by: legobeat <109787230+legobeat@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The lockfile hydration code is trying to unify similar lockfile entries:
However, when this file is read, the code was hydrating a single copy of the package, using a single name (others were dropped). This was causing strange behaviours when generating the hoisting, as the wrong package names were used to prevent incompatible version overrides.