Provides hotfix for type error caused by mismatch between the ControllerInstance generic constraint to the ChildControllers generic parameter of the ComposableController class.
As long as we're confident about the accuracy of the Controllers type, it should be safe to apply this @ts-expect-error, since we're overloading the ChildControllers generic parameter with known information about the input child controllers (Controllers[keyof Controllers]).
Required follow-up changes
ComposableController:
The BaseControllerV1Instance type needs to be fixed to accommodate controller versions used in mobile that inherit from BaseControllerV1.
Type 'PhishingController | AccountsController | AccountTrackerController | AddressBookController | ... 22 more ... | SwapsController' does not satisfy the constraint 'ControllerInstance'.
Type 'AccountTrackerController' is not assignable to type 'ControllerInstance'.
Type 'AccountTrackerController' is not assignable to type 'BaseControllerV1Instance'.
Types have separate declarations of a private property 'initialConfig'.ts(2344)
Add jsdoc for the ChildControllers optional generic parameter, making it clearer that a full list of controllers can be supplied by the consumer.
Fix LegacyComposableControllerStateConstraint to { [name: string]: Record<any, any> }, since index signature of interface types used for V1 state objects is not fixed to string.
Test on mobile engine whether runtime checks isBaseController() and isBaseControllerV1() are functioning as intended.
On mobile, upgrade the affected controllers to V2:
V2 upgrades are available for all of the following:
AccountTrackerController
NftController
TokenRatesController
TransactionController
Exceptions:
AssetsContractController: set to be upgraded to non-controller that uses messenger but doesn't inherit from BaseControllerV2. ComposableController will need to be updated to accommodated non-controllers
NftDetectionController: current release inherits from BaseControllerV2, but should be upgraded to non-controller.
[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 format if applicable
[ ] I’ve applied the right labels on the PR (see labeling guidelines). 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.
Description
Applies
ComposableController
API changes introduced to the controller in the following series of PRs:v5.0.0
)v6.0.0
)v6.0.0
)v6.0.2
)Provides hotfix for type error caused by mismatch between the
ControllerInstance
generic constraint to theChildControllers
generic parameter of theComposableController
class.Controllers
type, it should be safe to apply this@ts-expect-error
, since we're overloading theChildControllers
generic parameter with known information about the input child controllers (Controllers[keyof Controllers]
).Required follow-up changes
ComposableController
:BaseControllerV1Instance
type needs to be fixed to accommodate controller versions used in mobile that inherit fromBaseControllerV1
.ChildControllers
optional generic parameter, making it clearer that a full list of controllers can be supplied by the consumer.LegacyComposableControllerStateConstraint
to{ [name: string]: Record<any, any> }
, since index signature ofinterface
types used for V1 state objects is not fixed tostring
.isBaseController()
andisBaseControllerV1()
are functioning as intended.On mobile, upgrade the affected controllers to V2:
AccountTrackerController
NftController
TokenRatesController
TransactionController
AssetsContractController
: set to be upgraded to non-controller that uses messenger but doesn't inherit fromBaseControllerV2
.ComposableController
will need to be updated to accommodated non-controllersNftDetectionController
: current release inherits fromBaseControllerV2
, but should be upgraded to non-controller.SwapsController
: V2 upgrade unavailableRelated issues
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist