-
Notifications
You must be signed in to change notification settings - Fork 5.5k
feat: Bump @metamask/providers to ^20.0.0
#29936
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
feat: Bump @metamask/providers to ^20.0.0
#29936
Conversation
<!-- 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 link was pointing to a dead site, this fixes it to point to the correct URL. [](https://codespaces.new/MetaMask/metamask-extension/pull/29733?quickstart=1) ## **Related issues** N/A ## **Manual testing steps** 1. Go to https://github.com/MetaMask/metamask-extension/blob/develop/docs/README.md 2. Click the link to Developer Docs ## **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 - [ ] 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.
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
chainChanged, networkChanged`, 'connect', and 'disconnect', eventschainChanged, networkChanged, 'connect', and 'disconnect', events
chainChanged, networkChanged, 'connect', and 'disconnect', eventschainChanged, networkChanged, connect, and disconnect events
Builds ready [5eea5ea]
Page Load Metrics (1800 ± 92 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
chainChanged, networkChanged, connect, and disconnect events@metamask/providers to ^20.0.0
|
Reviewable, functional, but should probably be covered by two new E2E tests. I'll work on that tomorrow / this week |
Builds ready [0435868]
Page Load Metrics (1813 ± 66 ms)
Bundle size diffs [🚀 Bundle size reduced!]
|
itsyoboieltr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
…provider (#30111) <!-- 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** Previously the inpage provider would withhold events for chainChanged events (and property value updates, i.e. window.ethereum.chainId and .networkVersion) when the dapp's network was changed to an rpc endpoint that was unresponsive or did not support net_version. The dapp would instead receive a disconnect event. Now the inpage provider always emits chainChanged and networkChanged events (and exposes the correct values on window.ethereum.chainId and .networkVersion) when the selected network for the dapp has changed regardless of if the network being changed to is responsive or if it supports net_version requests. It does this by having the wallet send a loading for networkVersion when it cannot be resolved (same behavior as before) AND a new isConnected property in the metamask_getProviderState request and metamask_chainChanged events (these are different from the events emittted by window.ethereum). isConnected is derived from whether the NetworkController.state.networkMetadata[].status value is the Available constant. [](https://codespaces.new/MetaMask/metamask-extension/pull/30109?quickstart=1) ## **Related issues** See: #29936 Providers patch from commit (d919ab6b): MetaMask/providers#404 ## **Manual testing steps** ``` window.ethereum.on('accountsChanged', (data) => console.log('accountsChanged', data)) window.ethereum.on('chainChanged', (data) => console.log('chainChanged', data)) window.ethereum.on('networkChanged', (data) => console.log('networkChanged', data)) window.ethereum.on('connect', (data) => console.log('connect', data)) window.ethereum.on('disconnect', (data) => console.log('disconnect', data)) ``` 1. Go to a webpage. Enter the following in console 2. Change to Linea, see that the correct values are emitted for the `chainChanged` and `networkChanged` events 3. Change to Sepolia, see that the correct values are emitted for the `chainChanged` and `networkChanged` events 4. Change to a network that is non-responsive, see that `chainChanged` emits with the correct chainId, but `networkChanged` emits with a null value, AND there is a `disconnect` event emitted 5. Change back to a working network, see that the correct values are emitted for the `chainChanged` and `networkChanged` events, AND there is a `connect` event emitted with the new chainId 6. Do the same above with a responsive network that does not have `net_version` implemented. Se that the correct values are emitted for the `chainChanged`, that `networkChanged` emits null, and that there is no `disconnect` event emitted ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After**   ## **Pre-merge author checklist** - [ ] 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). - [ ] 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.
…provider (#30111) <!-- 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. --> Previously the inpage provider would withhold events for chainChanged events (and property value updates, i.e. window.ethereum.chainId and .networkVersion) when the dapp's network was changed to an rpc endpoint that was unresponsive or did not support net_version. The dapp would instead receive a disconnect event. Now the inpage provider always emits chainChanged and networkChanged events (and exposes the correct values on window.ethereum.chainId and .networkVersion) when the selected network for the dapp has changed regardless of if the network being changed to is responsive or if it supports net_version requests. It does this by having the wallet send a loading for networkVersion when it cannot be resolved (same behavior as before) AND a new isConnected property in the metamask_getProviderState request and metamask_chainChanged events (these are different from the events emittted by window.ethereum). isConnected is derived from whether the NetworkController.state.networkMetadata[].status value is the Available constant. [](https://codespaces.new/MetaMask/metamask-extension/pull/30109?quickstart=1) See: #29936 Providers patch from commit (d919ab6b): MetaMask/providers#404 ``` window.ethereum.on('accountsChanged', (data) => console.log('accountsChanged', data)) window.ethereum.on('chainChanged', (data) => console.log('chainChanged', data)) window.ethereum.on('networkChanged', (data) => console.log('networkChanged', data)) window.ethereum.on('connect', (data) => console.log('connect', data)) window.ethereum.on('disconnect', (data) => console.log('disconnect', data)) ``` 1. Go to a webpage. Enter the following in console 2. Change to Linea, see that the correct values are emitted for the `chainChanged` and `networkChanged` events 3. Change to Sepolia, see that the correct values are emitted for the `chainChanged` and `networkChanged` events 4. Change to a network that is non-responsive, see that `chainChanged` emits with the correct chainId, but `networkChanged` emits with a null value, AND there is a `disconnect` event emitted 5. Change back to a working network, see that the correct values are emitted for the `chainChanged` and `networkChanged` events, AND there is a `connect` event emitted with the new chainId 6. Do the same above with a responsive network that does not have `net_version` implemented. Se that the correct values are emitted for the `chainChanged`, that `networkChanged` emits null, and that there is no `disconnect` event emitted <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> <!-- [screenshots/recordings] -->   - [ ] 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). - [ ] 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. - [ ] 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.
adonesky1
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…provider (#30111) <!-- 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** Previously the inpage provider would withhold events for chainChanged events (and property value updates, i.e. window.ethereum.chainId and .networkVersion) when the dapp's network was changed to an rpc endpoint that was unresponsive or did not support net_version. The dapp would instead receive a disconnect event. Now the inpage provider always emits chainChanged and networkChanged events (and exposes the correct values on window.ethereum.chainId and .networkVersion) when the selected network for the dapp has changed regardless of if the network being changed to is responsive or if it supports net_version requests. It does this by having the wallet send a loading for networkVersion when it cannot be resolved (same behavior as before) AND a new isConnected property in the metamask_getProviderState request and metamask_chainChanged events (these are different from the events emittted by window.ethereum). isConnected is derived from whether the NetworkController.state.networkMetadata[].status value is the Available constant. [](https://codespaces.new/MetaMask/metamask-extension/pull/30109?quickstart=1) ## **Related issues** See: #29936 Providers patch from commit (d919ab6b): MetaMask/providers#404 ## **Manual testing steps** ``` window.ethereum.on('accountsChanged', (data) => console.log('accountsChanged', data)) window.ethereum.on('chainChanged', (data) => console.log('chainChanged', data)) window.ethereum.on('networkChanged', (data) => console.log('networkChanged', data)) window.ethereum.on('connect', (data) => console.log('connect', data)) window.ethereum.on('disconnect', (data) => console.log('disconnect', data)) ``` 1. Go to a webpage. Enter the following in console 2. Change to Linea, see that the correct values are emitted for the `chainChanged` and `networkChanged` events 3. Change to Sepolia, see that the correct values are emitted for the `chainChanged` and `networkChanged` events 4. Change to a network that is non-responsive, see that `chainChanged` emits with the correct chainId, but `networkChanged` emits with a null value, AND there is a `disconnect` event emitted 5. Change back to a working network, see that the correct values are emitted for the `chainChanged` and `networkChanged` events, AND there is a `connect` event emitted with the new chainId 6. Do the same above with a responsive network that does not have `net_version` implemented. Se that the correct values are emitted for the `chainChanged`, that `networkChanged` emits null, and that there is no `disconnect` event emitted ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After**   ## **Pre-merge author checklist** - [ ] 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). - [ ] 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.
ffmcgee725
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! 🚀
Builds ready [7b56a80]
Page Load Metrics (1655 ± 66 ms)
Bundle size diffs [🚀 Bundle size reduced!]
|
| method: NOTIFICATION_NAMES.unlockStateChanged, | ||
| params: { | ||
| isUnlocked: true, | ||
| accounts: this.getPermittedAccounts(origin), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm. There might still be a case where a dapp doesn't find out about an account change: when the accounts change before the wallet is marked as unlocked (e.g. if a change is triggered during that update, not sure exactly what the sequence of events is).
I see that we react to account changes below in this file, in _notifyAccountsChange, but we still have the if (this.isUnlocked()) { condition there. If we don't emit the new accounts either here or there, that seems like a problem.
Maybe we can continue emitting them here, or remove that isUnlocked condition in _notifyAccountsChange? Or am I misunderstanding how this works
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resolved in 7be8143 by removing isUnlocked condition in _notifyAccountsChange
Builds ready [820761d]
Page Load Metrics (1780 ± 106 ms)
Bundle size diffs [🚀 Bundle size reduced!]
|
…erState-chainChanged' into jl/send-isConnected-in-getProviderState-chainChanged
Builds ready [a457bc2]
Page Load Metrics (1523 ± 27 ms)
Bundle size diffs [🚀 Bundle size reduced!]
|
Builds ready [4868431]
Page Load Metrics (1718 ± 95 ms)
Bundle size diffs [🚀 Bundle size reduced!]
|
…onnected-in-getProviderState-chainChanged
adonesky1
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Builds ready [972574a]
Page Load Metrics (1693 ± 70 ms)
Bundle size diffs [🚀 Bundle size reduced!]
|
Gudahtt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Description
Previously the inpage provider would withhold events forchainChangedevents (and property value updates, i.e. window.ethereum.chainId and .networkVersion) when the dapp's network was changed to an rpc endpoint that was unresponsive or did not supportnet_version. The dapp would instead receive adisconnectevent.Now the inpage provider always emitschainChangedandnetworkChangedevents (and exposes the correct values on window.ethereum.chainId and .networkVersion) when the selected network for the dapp has changed regardless of if the network being changed to is responsive or if it supportsnet_versionrequests. It does this by having the wallet send aloadingfornetworkVersionwhen it cannot be resolved (same behavior as before) AND a newisConnectedproperty in themetamask_getProviderStaterequest andmetamask_chainChangedevents (these are different from the events emittted by window.ethereum).isConnectedis derived from whether theNetworkController.state.networkMetadata[].statusvalue is the Available constant.The above was handled in a separate PR here.
Additionally, this PR also no longer fires
metamask_unlockStateChangedevents to the inpage provider when the wallet is locked or unlocked. This has the downstream effect of makingwindow.ethereumno longer emit an empty array value for theaccountsChangedevent when the logic becomes locked.Related issues
Fixes: https://github.com/orgs/MetaMask/projects/146/views/1?filterQuery=label%3A%22team-wallet-api-platform%22+-status%3ABacklog&pane=issue&itemId=95374156&issue=MetaMask%7CMetaMask-planning%7C4039
Fixes: https://github.com/MetaMask/MetaMask-planning/issues/3853
Manual testing steps
For chainChanged/networkChanged/connect/disconnect:
networkChangedshould be emitted by the inpage provider withnulldisconnectevent should be fired ANDchainChangedandnetworkChangedshould fire as well if those values have changedconnectevent should be fired ANDchainChangedandnetworkChangedshould fire as well if those values have changeddisconnectevent should be fired andnetworkChangedshould return the previously cached valueFor accountsChanged:
accountsChangedevent emitted on the dapp consoleaccountsChangedevent emitted on the dapp consoleScreenshots/Recordings
Before
For chainChanged/networkChanged/connect/disconnect:
Screen.Recording.2025-01-27.at.4.13.46.PM.mov
For accountsChanged:
Screen.Recording.2025-02-03.at.3.32.32.PM.mov
After
For chainChanged/networkChanged/connect/disconnect:
Screen.Recording.2025-01-27.at.3.53.39.PM.mov
For accountsChanged:
Screen.Recording.2025-02-03.at.3.27.55.PM.mov
Pre-merge author checklist
Pre-merge reviewer checklist