Deprecate <ControllerName>:stateChange in favor of :stateChanged by mcmire · Pull Request #8187 · MetaMask/core

mcmire · 2026-03-11T21:01:13Z

Explanation

Since the :stateChange event was added to BaseController, we have added a guideline which suggests that engineers use past tense for all event names. This commit updates BaseController to align with that guideline. To maintain backward compatibility, we add a new event, :stateChanged, rather than replacing the existing event. We also deprecate subscribing to or delegating :stateChange by adding some custom ESLint rules.

References

We are about to add the :cacheUpdate to BaseDataService here, so I thought I'd deprecate BaseController:stateChange so we can add BaseDataService:cacheUpdated instead.

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 communicated my changes to consumers by updating changelogs for packages I've changed
I've introduced breaking changes in this PR and have prepared draft pull requests for clients and consumer packages to resolve them

Note

Medium Risk
Touches BaseController event publishing/typing and subscription cleanup, so regressions could impact any controller state listener. Backward compatibility is preserved by emitting both events, but consumers may see extra publishes.

Overview
Introduces a new controller state event name. BaseController now defines ControllerStateChangedEvent and treats controller events as a union of :stateChange and :stateChanged, registering initial payloads and publishing both events on update/applyPatches.

Deprecation enforcement and adoption guidance. ESLint adds no-restricted-syntax selectors to flag subscribing to or delegating *:*:stateChange, with accompanying updates to docs/guidelines and eslint-suppressions.json to account for existing usages.

Tests/changelog updated for dual emission. base-controller tests are refactored to validate behavior for both event names, some downstream tests loosen expectations around publish call ordering/counts, and packages/base-controller/CHANGELOG.md documents the addition and deprecation.

^{Written by Cursor Bugbot for commit 07cce7c. This will update automatically on new commits. Configure here.}

Since the `:stateChange` event was added to `BaseController`, we have added a guideline asking engineers to use past tense for all event names. This commit updates BaseController to align with that guideline. To achieve backward compatibility, we add a new event, `:stateChanged`, rather than replacing the existing event. We deprecate `:stateChange` by adding some custom ESLint rules.

packages/base-controller/src/BaseController.test.ts

mcmire · 2026-03-12T19:04:39Z

eslint.config.mjs

+      // replaces the rule entirely rather than merging arrays.
+      'no-restricted-syntax': [
+        'error',
+        ...getNoRestrictedSyntaxEntries(typescript),


Feels like there ought to be a better way to do this...

mcmire · 2026-03-12T19:11:28Z

packages/accounts-controller/src/AccountsController.test.ts

-        expect(messengerSpy).toHaveBeenNthCalledWith(
-          // 1. AccountsController:stateChange
-          2,
+        expect(messengerSpy).toHaveBeenCalledWith(


The name of this test indicates that all we care about is that the accountAdded event is published, so I not only fixed the test but changed it to reflect that.

mcmire · 2026-03-12T19:11:41Z

packages/accounts-controller/src/AccountsController.test.ts

-        expect(messengerSpy).toHaveBeenNthCalledWith(
-          // 1. AccountsController:stateChange
-          2,
+        expect(messengerSpy).toHaveBeenCalledWith(


Similar story here.

mcmire · 2026-03-12T19:12:19Z

packages/accounts-controller/src/AccountsController.test.ts

        accountsController.state.internalAccounts.selectedAccount,
      ).toStrictEqual(mockNonEvmAccount.id);

-      expect(messengerSpy.mock.calls).toHaveLength(2); // state change and then selectedAccountChange


Similar story; based on the test name it seems all we care about is that the selectedEvmAccountChange event is not published.

mcmire · 2026-03-12T19:14:09Z

packages/assets-controllers/src/RatesController/RatesController.test.ts


-      // check the 3rd call since the 2nd one is for the
-      // event stateChange
+      // Some of these calls are for state changes


Since there is a new state change event it changes the number of expected events.

mcmire · 2026-03-12T19:15:49Z

packages/base-controller/src/BaseController.ts

        >['type'] extends MessengerEvents<ControllerMessenger>['type']
        ? ControllerMessenger
-        : never
+        : ControllerStateChangedEvent<


Controllers must at least support :stateChange or :stateChanged. (We don't want to require that controllers support both :stateChange and :stateChanged, so we can't use ControllerEvents here anymore.)

docs/code-guidelines/controller-guidelines.md

package.json

eslint.config.mjs

…leOptions The getNoRestrictedSyntaxEntries function was redundant with the pre-existing collectExistingRuleOptions helper. It was also less robust: it used '?? []' instead of an Array.isArray guard, meaning it would throw a TypeError if an upstream config ever set the rule to a non-array value (e.g. 'off' or 0). Replace the call site with collectExistingRuleOptions('no-restricted-syntax', [typescript]) and remove the now-unused helper. Co-authored-by: Elliot Winkler <elliot.winkler@gmail.com>

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.}

cursor · 2026-03-31T05:38:35Z

packages/accounts-controller/src/AccountsController.test.ts

        accountsController.state.internalAccounts.selectedAccount,
      ).toStrictEqual(mockNonEvmAccount.id);

-      expect(messengerSpy.mock.calls).toHaveLength(2); // state change and then selectedAccountChange


Weakened test no longer verifies event is never published

Low Severity

The test "does not emit setSelectedEvmAccountChange if the account is non-EVM" previously had three assertions that together proved selectedEvmAccountChange was never published: a call-count check, a toHaveBeenLastCalledWith for the expected event, and not.toHaveBeenLastCalledWith for the unexpected event. The first two were removed, leaving only not.toHaveBeenLastCalledWith, which only checks the last call. With the new stateChanged event adding more published events, this assertion could pass even if selectedEvmAccountChange were erroneously published as a non-last event. Changing to not.toHaveBeenCalledWith would match the stated intent.

mcmire mentioned this pull request Mar 11, 2026

feat: Implement BaseDataService #8039

Merged

4 tasks

mcmire force-pushed the deprecate-state-change-event branch from dbd3929 to d8d71e5 Compare March 11, 2026 21:04

mcmire changed the title ~~Deprecate BaseController:stateChange in favor of :stateChanged~~ Deprecate <ControllerName>:stateChange in favor of :stateChanged Mar 11, 2026

mcmire force-pushed the deprecate-state-change-event branch from d8d71e5 to 41daada Compare March 12, 2026 18:51

mcmire marked this pull request as ready for review March 12, 2026 19:04

mcmire requested review from a team as code owners March 12, 2026 19:04

cursor bot reviewed Mar 12, 2026

View reviewed changes

packages/base-controller/src/BaseController.test.ts Outdated Show resolved Hide resolved

mcmire added 2 commits March 12, 2026 13:13

Tweak this test

1e8f1fd

Tweaks to changelog, BaseController

7f25d8e

mcmire commented Mar 12, 2026

View reviewed changes

Address Cursor feedback

d86747d

cursor bot reviewed Mar 12, 2026

View reviewed changes

docs/code-guidelines/controller-guidelines.md Show resolved Hide resolved

package.json Outdated Show resolved Hide resolved

mcmire added 3 commits March 12, 2026 14:09

Restore test:verbose script; add missing export and log in changelog

635f6df

Merge branch 'main' into deprecate-state-change-event

6e90197

Fix lint violation

f8abee3

cursor bot reviewed Mar 30, 2026

View reviewed changes

eslint.config.mjs Outdated Show resolved Hide resolved

cursoragent and others added 2 commits March 31, 2026 02:32

Inline the array

07cce7c

cursor bot reviewed Mar 31, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Deprecate <ControllerName>:stateChange in favor of :stateChanged#8187

Deprecate <ControllerName>:stateChange in favor of :stateChanged#8187
mcmire wants to merge 9 commits intomainfrom
deprecate-state-change-event

mcmire commented Mar 11, 2026 •

edited by cursor bot

Loading

Uh oh!

Uh oh!

mcmire Mar 12, 2026

Uh oh!

mcmire Mar 12, 2026

Uh oh!

mcmire Mar 12, 2026

Uh oh!

mcmire Mar 12, 2026

Uh oh!

mcmire Mar 12, 2026

Uh oh!

mcmire Mar 12, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cursor bot left a comment

Uh oh!

cursor bot Mar 31, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

mcmire commented Mar 11, 2026 • edited by cursor bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Explanation

References

Checklist

Uh oh!

Uh oh!

mcmire Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

mcmire Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

mcmire Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

mcmire Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

mcmire Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

mcmire Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cursor bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor bot Mar 31, 2026

Choose a reason for hiding this comment

Weakened test no longer verifies event is never published

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mcmire commented Mar 11, 2026 •

edited by cursor bot

Loading