-
-
Notifications
You must be signed in to change notification settings - Fork 23
chore: make @metamask/snap-simple-keyring-site publishable #163
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.
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
Merged
Conversation
This file contains hidden or 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
Mrtenz
reviewed
Sep 23, 2025
Mrtenz
approved these changes
Sep 23, 2025
FrederikBolding
pushed a commit
to MetaMask/snaps-registry
that referenced
this pull request
Oct 3, 2025
<!-- Thanks for your contribution! Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? Are there any issues or other links reviewers should consult to understand this pull request better? For instance: * Fixes #12345 * See: #67890 --> Context: The update to 2.0.0 doesn't change anything on the snap-simple-keyring. It's a major update, because we have made the snap-simple-keyring-site publishable, in order to use it in our e2e as a local server. See more here: - MetaMask/snap-simple-keyring#163 - MetaMask/metamask-extension#36557
seaona
added a commit
to MetaMask/snap-account-abstraction-keyring
that referenced
this pull request
Oct 8, 2025
…le (#215) <!-- Thanks for your contribution! Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? Are there any issues or other links reviewers should consult to understand this pull request better? For instance: * Fixes #12345 * See: #67890 --> ## Description Similar to what we did for the snap-simple-keyring-site, we are now doing the same for the account abstraction keyring site: we make the site publishable as an npm package, so we can then use that in our e2e as a local server, instead of going to the live site. See PRs for context: - MetaMask/snap-simple-keyring#163 - MetaMask/snap-simple-keyring#166 - MetaMask/snap-simple-keyring#168 - MetaMask/metamask-extension#36557 <!-- Are there any examples of this change being used in another repository? When considering changes to the MetaMask module template, it's strongly preferred that the change be experimented with in another repository first. This gives reviewers a better sense of how the change works, making it less likely the change will need to be reverted or adjusted later. --> <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Makes the site package publishable on npm and updates CI to cache the built `public` directory. > > - **Package (`packages/site/package.json`)**: > - Add `description`, `files: ["public"]`, and `publishConfig.access: "public"`; remove `private` to enable npm publishing. > - **CI (`.github/workflows/publish-release.yml`)**: > - Cache site build output by adding `./packages/site/public` to cache paths in `publish-release`, `publish-npm-dry-run`, and `publish-npm` jobs. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 331f5ce. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Maarten Zuidhoorn <maarten@zuidhoorn.com>
github-merge-queue bot
pushed a commit
to MetaMask/metamask-extension
that referenced
this pull request
Oct 9, 2025
…e requests to the site (#36557) <!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** This PR implements a localserver for the `snap-simple-keyring-site` instead of going to the live site, in our e2e, to avoid flakiness. This implies the following: - Removing all related live requests from the allowlist - Adding the `snap-simple-keyring-site` as an npm package -> this causes updates in our dependencies, lavamoat - This needs to be added to ignore the lavamoat error in our e2e - Lint needs to be fixed - Updating the snap simple keyring binaries, as the version was updated to 2.0.0 . The changes are just the result of running this `yarn update-snap-binary --snap-simple-keyring@2.0.0` - Usage in our e2e: ```javascript await withFixtures( { dapp: true, dappPaths: ['snap-simple-keyring-site'], ... ``` - I couldn't go to localhost directly, as localhost is not in the allowlist for that snap. So the final approach , suggested by @FrederikBolding , is to proxy the localhost, so we still see the origin as metamask.github so then we don't get errors, and we avoid adding localhost in the allowlist (this was a small security concern). Context: https://consensys.slack.com/archives/GN3SR3GNM/p1759489395046839 <img width="1758" height="1956" alt="image" src="https://github.com/user-attachments/assets/7e68d853-9797-4dfd-874d-f0a4c267ff46" /> PRs on the snap-simple-keyring-site for reference: MetaMask/snap-simple-keyring#163 MetaMask/snap-simple-keyring#166 MetaMask/snap-simple-keyring#168 [](https://codespaces.new/MetaMask/metamask-extension/pull/36557?quickstart=1) ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: ## **Related issues** Fixes: ## **Manual testing steps** 1. Tests should continue to work without going to the snap live site ## **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** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/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-extension/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. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Switches e2e to a locally served snap-simple-keyring site via proxy, updates tests/helpers, removes live allowlisted URLs, adds the site package, and adjusts LavaMoat/webpack policies with minor UI/type fixes. > > - **E2E/Testing**: > - Add local serving of `@metamask/snap-simple-keyring-site` (`helpers.js`), with `dappPaths` support and site-specific path resolution. > - Introduce `tests/account/snap-keyring-site-mocks.ts` to proxy `metamask.github.io` to localhost. > - Replace `mockSimpleKeyringSnap` with `mockSnapSimpleKeyringAndSite`; update tests to pass `dapp: true` and `dappPaths: ['snap-simple-keyring-site']` (and with `test-dapp` where needed). > - Remove live site URLs from `test/e2e/mock-e2e-allowlist.js`. > - Update snap binary headers for `snap-simple-keyring@2.0.0`. > - **Deps/Config**: > - Add `@metamask/snap-simple-keyring-site` to `package.json` and `.depcheckrc.yml`. > - Large `yarn.lock` refresh due to new dependency tree (MUI, Emotion, styled-components, etc.). > - **LavaMoat/Policies**: > - Adjust Browserify/Webpack policies (e.g., `@material-ui/...>clsx` path, `@popperjs/core` globals incl. `navigator.userAgentData`, browserslist mappings, Babel traverse entries). > - Update build-system policy package mappings and globals. > - **Build/Webpack**: > - Allow `devicePixelRatio` in LavaMoat scuttle exceptions and plugin test globals. > - **UI/Types**: > - Wrap `details` in `general-alert` with a fragment; cast `helpText` in `form-text-field`. > - Minor test typing updates (`ReactNodeLike`). > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 1d4109c. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: MetaMask Bot <metamaskbot@users.noreply.github.com>
NidhiKJha
pushed a commit
to MetaMask/metamask-extension
that referenced
this pull request
Oct 9, 2025
…e requests to the site (#36557) <!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** This PR implements a localserver for the `snap-simple-keyring-site` instead of going to the live site, in our e2e, to avoid flakiness. This implies the following: - Removing all related live requests from the allowlist - Adding the `snap-simple-keyring-site` as an npm package -> this causes updates in our dependencies, lavamoat - This needs to be added to ignore the lavamoat error in our e2e - Lint needs to be fixed - Updating the snap simple keyring binaries, as the version was updated to 2.0.0 . The changes are just the result of running this `yarn update-snap-binary --snap-simple-keyring@2.0.0` - Usage in our e2e: ```javascript await withFixtures( { dapp: true, dappPaths: ['snap-simple-keyring-site'], ... ``` - I couldn't go to localhost directly, as localhost is not in the allowlist for that snap. So the final approach , suggested by @FrederikBolding , is to proxy the localhost, so we still see the origin as metamask.github so then we don't get errors, and we avoid adding localhost in the allowlist (this was a small security concern). Context: https://consensys.slack.com/archives/GN3SR3GNM/p1759489395046839 <img width="1758" height="1956" alt="image" src="https://github.com/user-attachments/assets/7e68d853-9797-4dfd-874d-f0a4c267ff46" /> PRs on the snap-simple-keyring-site for reference: MetaMask/snap-simple-keyring#163 MetaMask/snap-simple-keyring#166 MetaMask/snap-simple-keyring#168 [](https://codespaces.new/MetaMask/metamask-extension/pull/36557?quickstart=1) ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: ## **Related issues** Fixes: ## **Manual testing steps** 1. Tests should continue to work without going to the snap live site ## **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** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/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-extension/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. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Switches e2e to a locally served snap-simple-keyring site via proxy, updates tests/helpers, removes live allowlisted URLs, adds the site package, and adjusts LavaMoat/webpack policies with minor UI/type fixes. > > - **E2E/Testing**: > - Add local serving of `@metamask/snap-simple-keyring-site` (`helpers.js`), with `dappPaths` support and site-specific path resolution. > - Introduce `tests/account/snap-keyring-site-mocks.ts` to proxy `metamask.github.io` to localhost. > - Replace `mockSimpleKeyringSnap` with `mockSnapSimpleKeyringAndSite`; update tests to pass `dapp: true` and `dappPaths: ['snap-simple-keyring-site']` (and with `test-dapp` where needed). > - Remove live site URLs from `test/e2e/mock-e2e-allowlist.js`. > - Update snap binary headers for `snap-simple-keyring@2.0.0`. > - **Deps/Config**: > - Add `@metamask/snap-simple-keyring-site` to `package.json` and `.depcheckrc.yml`. > - Large `yarn.lock` refresh due to new dependency tree (MUI, Emotion, styled-components, etc.). > - **LavaMoat/Policies**: > - Adjust Browserify/Webpack policies (e.g., `@material-ui/...>clsx` path, `@popperjs/core` globals incl. `navigator.userAgentData`, browserslist mappings, Babel traverse entries). > - Update build-system policy package mappings and globals. > - **Build/Webpack**: > - Allow `devicePixelRatio` in LavaMoat scuttle exceptions and plugin test globals. > - **UI/Types**: > - Wrap `details` in `general-alert` with a fragment; cast `helpText` in `form-text-field`. > - Minor test typing updates (`ReactNodeLike`). > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 1d4109c. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: MetaMask Bot <metamaskbot@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.
Makes @metamask/snap-simple-keyring-site publishable so we enable local serving in MetaMask e2e tests similar to @metamask/test-dapp.