refactor!: Replace SafeEventEmitterProvider with InternalProvider#6796
Merged
refactor!: Replace SafeEventEmitterProvider with InternalProvider#6796
SafeEventEmitterProvider with InternalProvider#6796Conversation
SafeEventEmitterProvider with InternalProvider
mcmire
reviewed
Oct 7, 2025
Contributor
There was a problem hiding this comment.
I think there's one thing with the proxies that we may want to adjust, but the rest looks good. Thank you for removing the lint warnings as well!
Can you:
- generate preview builds
- make a new branch on Extension, upgrade
network-controllerto the preview build, push the PR, and wait for CI to pass - build the extension locally, open the test dapp, and make sure you can still make network requests on different chains
- do the same thing for mobile
Let me know if you want to tag team this, I have a bit of time now.
packages/selected-network-controller/src/SelectedNetworkController.ts
Outdated
Show resolved
Hide resolved
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Member
Author
|
@metamaskbot publish-preview |
Contributor
|
Preview builds have been published. See these instructions for more information about preview builds. Expand for full list of packages and versions. |
Member
Author
cc: @mcmire |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
c48651b to
2caa429
Compare
Co-authored-by: Elliot Winkler <elliot.winkler@gmail.com>
2caa429 to
f0dbedf
Compare
| * @deprecated Use {@link InternalProvider} instead. | ||
| */ | ||
| type SafeEventEmitterProvider = InternalProvider; | ||
| const SafeEventEmitterProvider = InternalProvider; |
Member
There was a problem hiding this comment.
What is the reasoning for keeping the alias around if we are removing it everywhere?
Member
Author
There was a problem hiding this comment.
Builds / tests fail because something monorepo-external uses SafeEventEmitterProvider and removing it completely blows up. My suspicion is that all library uses of it need to be removed, then we can delete the deprecated alias completely.
|
|
||
| this.messenger.registerActionHandler( | ||
| // TODO: Either fix this lint violation or explain why it's necessary to ignore. | ||
| // eslint-disable-next-line @typescript-eslint/restrict-template-expressions |
Member
There was a problem hiding this comment.
Why are all of these no longer required? 🤔
Member
Author
There was a problem hiding this comment.
My suspicion is that they weren't required prior to this PR, and the manner in which I linted the package (yarn eslint --fix packages/... from root) removed a bunch of unused disable directives.
Contributor
There was a problem hiding this comment.
When these lint violations appeared, at the time we were using an older version of the typescript-eslint packages. When we upgraded, they went away.
mcmire
approved these changes
Oct 31, 2025
| import { type JsonRpcRequest, type Json } from '@metamask/utils'; | ||
| import { BrowserProvider } from 'ethers'; | ||
| import { promisify } from 'util'; | ||
| // eslint-disable-next-line import-x/namespace |
Contributor
There was a problem hiding this comment.
Ugh, I'm not sure why we have this lint rule, it doesn't make sense to me. Oh well...
|
|
||
| this.messenger.registerActionHandler( | ||
| // TODO: Either fix this lint violation or explain why it's necessary to ignore. | ||
| // eslint-disable-next-line @typescript-eslint/restrict-template-expressions |
Contributor
There was a problem hiding this comment.
When these lint violations appeared, at the time we were using an older version of the typescript-eslint packages. When we upgraded, they went away.
jiexi
approved these changes
Oct 31, 2025
ejwessel
approved these changes
Oct 31, 2025
Member
Author
|
I am bypassing rules to merge this without a review from @MetaMask/confirmations. Their review requirement was triggered by changes to |
7 tasks
github-merge-queue bot
pushed a commit
to MetaMask/metamask-extension
that referenced
this pull request
Mar 23, 2026
## **Description** Bump the following packages: ### `@metamask/chain-agnostic-permission` `^1.3.0` → `^1.4.0` [Changelog](https://github.com/MetaMask/core/blob/main/packages/chain-agnostic-permission/CHANGELOG.md#140) - **Added:** `Bip122AccountChangedNotifications` property in `KnownSessionProperties` enum ([#7537](MetaMask/core#7537)) - **Changed:** Remove `@metamask/network-controller` dependency ([#7561](MetaMask/core#7561)) - **Changed:** Dependency bumps (`@metamask/utils`, `@metamask/controller-utils`, `@metamask/permission-controller`) ### `@metamask/multichain-api-middleware` `^1.2.5` → `^1.2.7` [Changelog](https://github.com/MetaMask/core/blob/main/packages/multichain-api-middleware/CHANGELOG.md#127) - **v1.2.7:** Dependency bumps (`network-controller` ^30.0.0, `json-rpc-engine`, `multichain-transactions-controller`, `controller-utils`) - **v1.2.6 Fixed:** `wallet_revokeSession` to handle cases where `params` is not provided ([#7551](MetaMask/core#7551)) - **v1.2.6 Changed:** Dependency bumps (`json-rpc-engine`, `utils`, `network-controller`, `controller-utils`, `permission-controller`, `chain-agnostic-permission`) ### `@metamask/ppom-validator` `0.39.0` → `0.39.1` [Changelog](https://github.com/MetaMask/ppom-validator/blob/main/CHANGELOG.md#0391) - **Changed:** Move `@metamask/network-controller` from peer to direct dependency and bump ^25.0.0 → ^30.0.0 ([#254](MetaMask/ppom-validator#254)) - **Changed:** Bump `@metamask/utils` from `^9.2.1` to `^11.0.0` ([#254](MetaMask/ppom-validator#254)) ### `@metamask/selected-network-controller` `^25.0.0` → `^26.0.3` [Changelog](https://github.com/MetaMask/core/blob/main/packages/selected-network-controller/CHANGELOG.md#2600) - **v26.0.0 BREAKING:** Use `InternalProvider` instead of `SafeEventEmitterProvider` ([#6796](MetaMask/core#6796)) - **v26.0.0 BREAKING:** Bump `@metamask/network-controller` from ^25.0.0 to ^26.0.0 ([#7202](MetaMask/core#7202)) - **v26.0.1:** Move peer dependencies to direct dependencies (`network-controller`, `permission-controller`) ([#7209](MetaMask/core#7209)) - **v26.0.2–v26.0.3:** Dependency bumps (`network-controller` to ^30.0.0, `json-rpc-engine`) No code changes required — breaking changes are internal to the packages and don't affect extension usage. All dependency versions are already compatible. ## **Changelog** CHANGELOG entry: null ## **Related issues** Fixes: N/A ## **Manual testing steps** 1. Build the extension and verify it runs correctly 2. Confirm no type errors are introduced ## **Screenshots/Recordings** N/A — dependency-only changes ## **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 - [ ] 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-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] > **Medium Risk** > Dependency-only update, but it upgrades core networking/permission-related packages (e.g., `@metamask/selected-network-controller` and transitive `@metamask/network-controller`/RPC middleware), which can subtly affect session/network behavior at runtime. > > **Overview** > Bumps several MetaMask core dependencies: `@metamask/chain-agnostic-permission` to `^1.4.0`, `@metamask/multichain-api-middleware` to `^1.2.7`, `@metamask/ppom-validator` to `0.39.1`, and `@metamask/selected-network-controller` to `^26.0.3`. > > Updates `yarn.lock` accordingly, including transitive shifts such as `@metamask/network-controller` moving to `^30.0.0` in dependent packages and related RPC middleware/util version bumps. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 133258b. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
Explanation
As of #6328
SafeEventEmitterProviderno longer used theEventEmitterAPI, making its name misleading. Consequently, this PR renames it toInternalProviderand stops extendingSafeEventEmitter. This new name better reflects its use as an internal-only provider that does not conform to any particular standard.SafeEventEmitterProvideris still exported as a deprecated alias ofInternalProviderin order to circumvent build failures due to repo-external packages that use this alias. Once we have updated external packages to useInternalProviderinstead, the old alias can be removed.References
Closes #6594
Checklist
Note
Renames and replaces SafeEventEmitterProvider with InternalProvider across packages, updates proxies and types, adds a deprecated alias, and adjusts tests/changelogs accordingly.
InternalProvider(no longer extends SafeEventEmitter); updateproviderFromEngine/providerFromMiddlewareto return it.SafeEventEmitterProvideras a deprecated alias; update tests/exports; drop@metamask/safe-event-emitterdep.InternalProviderinblock-ref,retryOnEmpty, andproviderAsMiddleware.InternalProviderinPollingBlockTrackerand tests; update CHANGELOG (BREAKING).InternalProvideracross types/clients.createSwappableProxyfor provider proxies; keepcreateEventEmitterProxyfor block tracker; adjust handlers/comments and tests.createSwappableProxyfor provider proxy; align withInternalProvider; update tests.FakeProvider/FakeBlockTrackerand various tests toInternalProvider.Written by Cursor Bugbot for commit e1faf35. This will update automatically on new commits. Configure here.