Publish preview builds to NPM instead of GitHub by mcmire · Pull Request #1622 · MetaMask/core

mcmire · 2023-08-22T21:50:15Z

Explanation

Currently, when publishing core packages, we use NPM for "production" releases but GitHub Package Registry for "preview" builds. This works to keep the list of production versions free of clutter and mitigate the risk of publishing preview builds as production releases accidentally, but it also introduces a major pain point.

Usually in order to access a package with a preview build, the developer must instruct Yarn to use GitHub Package Registry instead of NPM along with an authentication token, and then update the version of the package in the manifest to use the preview version. If, however, the package introduces a new transitive dependency that is not available in preview form, the developer must undo the changes to the Yarn configuration or even temporarily add the dependency explicitly to the project in order to fully install it. In some cases, these steps don't work at all.

To remove this pain point, this commit changes how preview builds are published so that instead of using a separate registry, we use a separate NPM scope. This preserves the separation between preview builds and non-preview builds, and it means that the developer can mix non-preview and preview versions in the manifest without needing to reconfigure Yarn.

References

Fixes #1075.
Fixes #1585.

Changelog

(No consumer-facing changes)

Checklist

I've updated the test suite for new or updated code as appropriate
I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
I've highlighted breaking changes using the "BREAKING" category above as appropriate

mcmire · 2023-08-22T21:54:02Z

@metamaskbot publish-preview

mcmire · 2023-08-22T21:57:05Z

@metamaskbot publish-preview

mcmire · 2023-08-22T22:56:57Z

@metamaskbot publish-preview

mcmire · 2023-08-22T23:02:52Z

@metamaskbot publish-preview

mcmire · 2023-08-23T01:24:17Z

@metamaskbot publish-preview

mcmire · 2023-08-23T01:34:30Z

@metamaskbot publish-preview

mcmire · 2023-08-23T01:50:20Z

@metamaskbot publish-preview

mcmire · 2023-08-23T15:34:39Z

scripts/prepare-preview-builds.sh

+done < <(yarn workspaces list --no-private --json | jq --slurp --raw-output 'map(select(.location != ".")) | map([.location, .name]) | map(@tsv) | .[]')
+
+echo "Installing dependencies..."
+yarn install --no-immutable


Yarn will use --immutable by default when it detects it's being run in a CI environment. We have to turn this off because we have to regenerate the lockfile (since we are changing the names of packages etc.). If we don't do this then we won't be able to run any Yarn commands following this script.

mcmire · 2023-08-23T15:37:44Z

package.json

    "prepack": "./scripts/prepack.sh",
-    "prepare-preview-builds": "yarn workspaces foreach --parallel run prepare-manifest:preview",
-    "publish-previews": "yarn workspaces foreach --parallel run publish:preview",
+    "prepare-preview-builds": "scripts/prepare-preview-builds.sh",


We let the script take care of applying changes across the monorepo instead of adding a script to each package and using yarn workspaces foreach. We do this because once we change a manifest we have to run yarn install in order to run another Yarn command. yarn workspaces foreach is not intelligent; it's more or less a shortcut for:

for each workspace in the monorepo yarn <whatever command you want to run> end

And so if we use yarn workspaces foreach, the first package in the list of workspaces will get updated but none of the other packages will get updated because Yarn will fail.

Currently, when publishing core packages, we use NPM for "production" releases but GitHub Package Registry for "preview" builds. This works to keep the list of production versions free of clutter and mitigate the risk of publishing preview builds as production releases accidentally, but it also introduces a major pain point. Usually in order to access a package with a preview build, the developer must instruct Yarn to use GitHub Package Registry instead of NPM along with an authentication token, and then update the version of the package in the manifest to use the preview version. If, however, the package introduces a new transitive dependency that is not available in preview form, the developer must undo the changes to the Yarn configuration or even temporarily add the dependency explicitly to the project in order to fully install it. In some cases, these steps don't work at all. To remove this pain point, this commit changes how preview builds are published so that instead of using a separate registry, we use a separate NPM scope. This preserves the separation between preview builds and non-preview builds, and it means that the developer can mix non-preview and preview versions in the manifest without needing to reconfigure Yarn.

Gudahtt · 2023-08-23T17:28:59Z

We'll need to update contributing.md as well, to remove the references to the GitHub npm registry

package.json

Gudahtt · 2023-08-23T18:03:51Z

Looks great! Once the docs are updated I think it will be ready

Co-authored-by: Mark Stacey <markjstacey@gmail.com>

Gudahtt

LGTM!

Currently, when publishing core packages, we use NPM for "production" releases but GitHub Package Registry for "preview" builds. This works to keep the list of production versions free of clutter and mitigate the risk of publishing preview builds as production releases accidentally, but it also introduces a major pain point. Usually in order to access a package with a preview build, the developer must instruct Yarn to use GitHub Package Registry instead of NPM along with an authentication token, and then update the version of the package in the manifest to use the preview version. If, however, the package introduces a new transitive dependency that is not available in preview form, the developer must undo the changes to the Yarn configuration or even temporarily add the dependency explicitly to the project in order to fully install it. In some cases, these steps don't work at all. To remove this pain point, this commit changes how preview builds are published so that instead of using a separate registry, we use a separate NPM scope. This preserves the separation between preview builds and non-preview builds, and it means that the developer can mix non-preview and preview versions in the manifest without needing to reconfigure Yarn. Co-authored-by: Mark Stacey <markjstacey@gmail.com>

## **Description** CI has a step where it sets up an alternative registry for `@metamask`- scoped packages specifically for draft PRs. This was intended to support the older stype of `MetaMask/core` preview builds, which used an alternative npm registry. Preview builds today now use the normal npm registry, so this step is no longer needed. ## **Related issues** Originally added here: #5270 Some related changes here: #5807 (these were already removed) The old preview build method was replaced here: MetaMask/core#1622 ## **Manual testing steps** N/A ## **Screenshots/Recordings** N/A ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile 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** - [ ] 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.

mcmire force-pushed the use-different-npm-scope branch 2 times, most recently from f75235e to e2b730a Compare August 23, 2023 02:03

mcmire commented Aug 23, 2023

View reviewed changes

mcmire marked this pull request as ready for review August 23, 2023 15:37

mcmire requested review from a team as code owners August 23, 2023 15:37

mcmire force-pushed the use-different-npm-scope branch from e2b730a to 9cd5809 Compare August 23, 2023 15:38

mcmire marked this pull request as draft August 23, 2023 15:39

Gudahtt reviewed Aug 23, 2023

View reviewed changes

package.json Outdated Show resolved Hide resolved

mcmire and others added 3 commits August 23, 2023 12:21

Use relative paths

d15734d

Co-authored-by: Mark Stacey <markjstacey@gmail.com>

Update contributing instructions

256826f

Allow NPM scope to be customized

49b1c3f

Gudahtt approved these changes Aug 23, 2023

View reviewed changes

mcmire marked this pull request as ready for review August 23, 2023 20:15

Mrtenz approved these changes Aug 23, 2023

View reviewed changes

mcmire merged commit f34f754 into main Aug 23, 2023

mcmire deleted the use-different-npm-scope branch August 23, 2023 20:27

Gudahtt mentioned this pull request Aug 24, 2023

Release 73.0.0 #1630

Merged

Gudahtt mentioned this pull request Oct 11, 2024

chore: Remove obsolete CI step MetaMask/metamask-mobile#11756

Merged

7 tasks

Uh oh!

Conversation

mcmire commented Aug 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Explanation

References

Changelog

Checklist

Uh oh!

mcmire commented Aug 22, 2023

Uh oh!

mcmire commented Aug 22, 2023

Uh oh!

mcmire commented Aug 22, 2023

Uh oh!

mcmire commented Aug 22, 2023

Uh oh!

mcmire commented Aug 23, 2023

Uh oh!

mcmire commented Aug 23, 2023

Uh oh!

mcmire commented Aug 23, 2023

Uh oh!

mcmire Aug 23, 2023

Choose a reason for hiding this comment

Uh oh!

mcmire Aug 23, 2023

Choose a reason for hiding this comment

Uh oh!

Gudahtt commented Aug 23, 2023

Uh oh!

Uh oh!

Gudahtt commented Aug 23, 2023

Uh oh!

Gudahtt left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mcmire commented Aug 22, 2023 •

edited

Loading