fix(network-connection-banner-controller): subscribe to ClientController:stateChange - #9893
Conversation
…ler:stateChange The controller subscribed to ClientController:stateChanged using a locally defined event type, but @metamask/client-controller only declares and exports ClientController:stateChange. Clients that delegate the exported event (mobile) never deliver UI open state, so the controller never starts and the banner never shows.
Both clients delegate ClientController:stateChanged today and BaseController publishes both names, so the banner does work at runtime. The defect is the undeclared event type, not a dead subscription.
|
@metamaskbot publish-preview |
| this.messenger.subscribe( | ||
| 'ClientController:stateChanged', | ||
| // eslint-disable-next-line no-restricted-syntax -- awaiting upstream :stateChanged migration | ||
| 'ClientController:stateChange', |
There was a problem hiding this comment.
Why is this necessary? Does ClientController not publish both?
There was a problem hiding this comment.
Because it's marked as deprecated.
There was a problem hiding this comment.
If remove it we get
Subscribing to ':stateChange' events is deprecated. Use ':stateChanged' instead.eslint[no-restricted-syntax](https://eslint.org/docs/latest/rules/no-restricted-syntax)
There was a problem hiding this comment.
I wasn't referring to the eslint comment. I am curious why we are making this change in the first place and why we can't use :stateChanged in this controller? Is it not published by ClientController?
There was a problem hiding this comment.
No it's not defined by ClientController because it still uses :stateChange https://github.com/MetaMask/core/blob/main/packages/client-controller/src/ClientController.ts#L82
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
Explanation
NetworkConnectionBannerControllersubscribes toClientController:stateChangedusing an event type it defines locally.@metamask/client-controlleronly declares and exportsClientController:stateChange.It works at runtime because
BaseControllerpublishes both names. This is a contract cleanup, not a behavior fix.Now the controller uses the exported
ClientControllerStateChangeEvent, dropping the duplicated local type and matching the three other upstream subscriptions in the file.Breaking: clients must delegate
ClientController:stateChangeinstead ofClientController:stateChanged, otherwise the controller stops receiving UI open state and the banner never shows. Both extension and mobile need the one line swap.References
Checklist
Note
Cursor Bugbot is generating a summary for commit ec8b9d7. Configure here.