-
-
Notifications
You must be signed in to change notification settings - Fork 188
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
[base-controller
] Add RestrictedControllerMessenger
tests for runtime error handling
#2058
[base-controller
] Add RestrictedControllerMessenger
tests for runtime error handling
#2058
Conversation
base-controller
] Add RestrctedControllerMessenger
tests for runtime error handlingbase-controller
] Add RestrictedControllerMessenger
tests for runtime error handling
1a1ca55
to
caa5c6d
Compare
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.
One thing, but otherwise looks good.
// @ts-expect-error suppressing to test runtime error handling | ||
restrictedControllerMessenger.registerActionHandler( | ||
'CountController:count', | ||
); |
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.
Doesn't registerActionHandler
take two arguments? Should the // @ts-expect-error
be added above the action not above the method call?
// @ts-expect-error suppressing to test runtime error handling | |
restrictedControllerMessenger.registerActionHandler( | |
'CountController:count', | |
); | |
restrictedControllerMessenger.registerActionHandler( | |
// @ts-expect-error suppressing to test runtime error handling | |
'CountController:count', | |
() => { | |
// do nothing | |
} | |
); |
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.
Good point! Fixed here 42a614e
Co-authored-by: Elliot Winkler <[email protected]>
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.
Great!
Explanation
RestrictedControllerMessenger
methods were not being tested on the basis of compile-time checks: "Branches unreachable with valid types".@ts-expect-error
is used to suppress the compile-time type errors to simulate a JavaScript environment.References
base-controller
] AddRestrictedControllerMessenger
tests for runtime error handling #2059base-controller
] Enforce thatRestrictedControllerMessenger
is not initialized with internal actions/events in allow lists #2051 (comment)Changelog
@metamask/base-controller
Checklist