-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
feat(expect, @jest/expect-utils): allow isA
utility to take a type argument
#13355
Conversation
@@ -262,10 +262,7 @@ export function isImmutableUnorderedSet(maybeSet: any) { | |||
} | |||
|
|||
export function isImmutableList(maybeList: any) { | |||
return !!( |
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.
This and following changes comes from Prettier. CI does not catch them, because of /* eslint-disable */
in this file.
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.
ah, then we should make sure to include this file in https://github.com/facebook/jest/blob/8759d63787b832af1fc9ac0ec2cc57b6f325ba9a/package.json#L103
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.
Just check, I will send a separate PR removing that comment. Seems doable.
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.
we have purposefully not been running eslint on it to keep the diff from the jasmine equivalent down.
That might not be relevant 6-7 years later
asymmetricMatch(other: unknown) { | ||
const result = isA<string>('String', other) && other.includes(this.sample); |
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.
This is just an internal type, user facing external types do not change. The point is that internally other
is of type unknown
in all the matchers. As an illustration see this test (all matchers have similar tests and that is a problem for typechecking):
At first it sounded strange, but reasons are very well explained in #7107
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.
ah, very nice!
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Summary
While working on typechecks of
expect
types, I found out that it would be useful to allowisA
utility to take a type argument. This changes turns it into a type guard and could be used for type narrowing.Test plan
Type tests are added.