Skip to content

Commit b3f16e6

Browse files
authored
fix: Messenger type errors in ComposableController (#6904)
## Explanation <!-- 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 changes whose purpose might not obvious to those unfamiliar with the domain? * If your primary goal was to update one package but you found you had to update another one along the way, why did you do so? * If you had to upgrade a dependency, why did you do so? --> - Fix incompatibility of `ChildControllerStateChangeEvents` type with `BaseController` (when used in the `Events` type argument of `ComposableControllerMessenger`) by removing unnecessary nested logic from definition. - Update generic parameter names `ControllerName` and `ControllerState` to `ChildControllerName`, `ChildControllerState` for reduced ambiguity. ## References <!-- Are there any issues that this pull request is tied to? Are there other links that reviewers should consult to understand these changes better? Are there client or consumer pull requests to adopt any breaking changes? For example: * Fixes #12345 * Related to #67890 --> - Branches from #6710 ## 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](https://github.com/MetaMask/core/tree/main/docs/contributing.md#updating-changelogs), highlighting breaking changes as necessary - [ ] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Simplifies `ChildControllerStateChangeEvents` and types the messenger `subscribe` call to fix `ComposableController` Messenger type errors; updates changelog. > > - **Composable Controller**: > - **Types**: Simplify `ChildControllerStateChangeEvents` by removing nested conditional logic and renaming generics to `ChildControllerName`/`ChildControllerState`. > - **Messenger**: Type the `subscribe` call for child `stateChange` events, updating composed state with the received child state (with an inline ts-expect-error to bypass an unnecessary overload constraint). > - **Docs**: > - Update `packages/composable-controller/CHANGELOG.md` with a Fixed entry describing the type compatibility change. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit b3444fb. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent ec56365 commit b3f16e6

File tree

4 files changed

+23
-18
lines changed

4 files changed

+23
-18
lines changed

.vscode/settings.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"typescript.tsdk": "node_modules/typescript/lib"
3+
}

eslint-warning-thresholds.json

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -107,9 +107,6 @@
107107
"packages/composable-controller/src/ComposableController.test.ts": {
108108
"import-x/namespace": 3
109109
},
110-
"packages/composable-controller/src/ComposableController.ts": {
111-
"@typescript-eslint/no-unused-vars": 1
112-
},
113110
"packages/controller-utils/jest.environment.js": {
114111
"n/prefer-global/text-encoder": 1,
115112
"n/prefer-global/text-decoder": 1,

packages/composable-controller/CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Fixed
11+
12+
- Resolve incompatibility of `ChildControllerStateChangeEvents` type with `BaseController` (when used in the `Events` type argument of `ComposableControllerMessenger`) by removing unnecessary nested logic from definition ([#6904](https://github.com/MetaMask/core/pull/6904))
13+
- Also update generic parameter names `ControllerName` and `ControllerState` to `ChildControllerName`, `ChildControllerState` for reduced ambiguity.
14+
1015
## [11.1.0]
1116

1217
### Added

packages/composable-controller/src/ComposableController.ts

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -70,12 +70,10 @@ export type ChildControllerStateChangeEvents<
7070
ComposableControllerState extends ComposableControllerStateConstraint,
7171
> =
7272
ComposableControllerState extends Record<
73-
infer ControllerName extends string,
74-
infer ControllerState
73+
infer ChildControllerName extends string,
74+
infer ChildControllerState extends StateConstraint
7575
>
76-
? ControllerState extends StateConstraint
77-
? ControllerStateChangeEvent<ControllerName, ControllerState>
78-
: never
76+
? ControllerStateChangeEvent<ChildControllerName, ChildControllerState>
7977
: never;
8078

8179
/**
@@ -184,16 +182,18 @@ export class ComposableController<
184182
throw new Error(`${name} - ${INVALID_CONTROLLER_ERROR}`);
185183
}
186184
try {
187-
this.messenger.subscribe(
188-
`${name}:stateChange`,
189-
(childState: StateConstraint) => {
190-
this.update((state) => {
191-
// Type assertion is necessary for property assignment to a generic type. This does not pollute or widen the type of the asserted variable.
192-
// @ts-expect-error "Type instantiation is excessively deep"
193-
(state as ComposableControllerStateConstraint)[name] = childState;
194-
});
195-
},
196-
);
185+
this.messenger.subscribe<
186+
// The type intersection with "ComposableController:stateChange" is added by one of the `Messenger.subscribe` overloads, but that constraint is unnecessary here,
187+
// since this method only subscribes the messenger to child controller `stateChange` events.
188+
// @ts-expect-error "Type '`${string}:stateChange`' is not assignable to parameter of type '"ComposableController:stateChange" & ChildControllerStateChangeEvents<ComposableControllerState>["type"]'."
189+
ChildControllerStateChangeEvents<ComposableControllerState>['type']
190+
>(`${name}:stateChange`, (childState: StateConstraint) => {
191+
this.update((state) => {
192+
// Type assertion is necessary for property assignment to a generic type. This does not pollute or widen the type of the asserted variable.
193+
// @ts-expect-error "Type instantiation is excessively deep"
194+
(state as ComposableControllerStateConstraint)[name] = childState;
195+
});
196+
});
197197
} catch (error: unknown) {
198198
// False negative. `name` is a string type.
199199
// eslint-disable-next-line @typescript-eslint/restrict-template-expressions

0 commit comments

Comments
 (0)