Skip to content
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

Merged
merged 2 commits into from
Oct 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
### Features

- `[jest-core]` Enable testResultsProcessor to be async ([#13343](https://github.com/facebook/jest/pull/13343))
- `[expect, @jest/expect-utils]` Allow `isA` utility to take a type argument ([#13355](https://github.com/facebook/jest/pull/13355))

### Fixes

Expand Down
12 changes: 12 additions & 0 deletions packages/expect-utils/__typetests__/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"extends": "../../../tsconfig.json",
"compilerOptions": {
"composite": false,
"noUnusedLocals": false,
"noUnusedParameters": false,
"skipLibCheck": true,

"types": []
},
"include": ["./**/*"]
}
38 changes: 38 additions & 0 deletions packages/expect-utils/__typetests__/utils.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

import {expectType} from 'tsd-lite';
import {isA} from '@jest/expect-utils';

// isA

expectType<boolean>(isA('String', 'default'));
expectType<boolean>(isA<number>('Number', 123));

const sample = {} as unknown;

expectType<unknown>(sample);

if (isA('String', sample)) {
expectType<unknown>(sample);
}

if (isA<string>('String', sample)) {
expectType<string>(sample);
}

if (isA<number>('Number', sample)) {
expectType<number>(sample);
}

if (isA<Map<unknown, unknown>>('Map', sample)) {
expectType<Map<unknown, unknown>>(sample);
}

if (isA<Set<unknown>>('Set', sample)) {
expectType<Set<unknown>>(sample);
}
4 changes: 3 additions & 1 deletion packages/expect-utils/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,10 @@
"jest-get-type": "workspace:^"
},
"devDependencies": {
"@tsd/typescript": "~4.8.2",
"immutable": "^4.0.0",
"jest-matcher-utils": "workspace:^"
"jest-matcher-utils": "workspace:^",
"tsd-lite": "^0.6.0"
},
"engines": {
"node": "^14.15.0 || ^16.10.0 || >=18.0.0"
Expand Down
15 changes: 4 additions & 11 deletions packages/expect-utils/src/jasmineUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ function hasKey(obj: any, key: string) {
return Object.prototype.hasOwnProperty.call(obj, key);
}

export function isA(typeName: string, value: unknown) {
export function isA<T>(typeName: string, value: unknown): value is T {
return Object.prototype.toString.apply(value) === '[object ' + typeName + ']';
}

Expand Down Expand Up @@ -262,10 +262,7 @@ export function isImmutableUnorderedSet(maybeSet: any) {
}

export function isImmutableList(maybeList: any) {
return !!(
Copy link
Contributor Author

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

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.

Copy link
Member

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

maybeList &&
maybeList[IS_LIST_SENTINEL]
);
return !!(maybeList && maybeList[IS_LIST_SENTINEL]);
}

export function isImmutableOrderedKeyed(maybeKeyed: any) {
Expand All @@ -276,7 +273,6 @@ export function isImmutableOrderedKeyed(maybeKeyed: any) {
);
}


export function isImmutableOrderedSet(maybeSet: any) {
return !!(
maybeSet &&
Expand All @@ -286,8 +282,5 @@ export function isImmutableOrderedSet(maybeSet: any) {
}

export function isImmutableRecord(maybeSet: any) {
return !!(
maybeSet &&
maybeSet[IS_RECORD_SYMBOL]
);
}
return !!(maybeSet && maybeSet[IS_RECORD_SYMBOL]);
}
7 changes: 5 additions & 2 deletions packages/expect-utils/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ export const iterableEquality = (
if (a.size !== undefined) {
if (a.size !== b.size) {
return false;
} else if (isA('Set', a) || isImmutableUnorderedSet(a)) {
} else if (isA<Set<unknown>>('Set', a) || isImmutableUnorderedSet(a)) {
let allFound = true;
for (const aValue of a) {
if (!b.has(aValue)) {
Expand All @@ -206,7 +206,10 @@ export const iterableEquality = (
aStack.pop();
bStack.pop();
return allFound;
} else if (isA('Map', a) || isImmutableUnorderedKeyed(a)) {
} else if (
isA<Map<unknown, unknown>>('Map', a) ||
isImmutableUnorderedKeyed(a)
) {
let allFound = true;
for (const aEntry of a) {
if (
Expand Down
15 changes: 8 additions & 7 deletions packages/expect/src/asymmetricMatchers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ class ArrayContaining extends AsymmetricMatcher<Array<unknown>> {
super(sample, inverse);
}

asymmetricMatch(other: Array<unknown>) {
asymmetricMatch(other: unknown) {
if (!Array.isArray(this.sample)) {
throw new Error(
`You must provide an array to ${this.toString()}, not '${typeof this
Expand Down Expand Up @@ -257,8 +257,8 @@ class StringContaining extends AsymmetricMatcher<string> {
super(sample, inverse);
}

asymmetricMatch(other: string) {
const result = isA('String', other) && other.includes(this.sample);
asymmetricMatch(other: unknown) {
const result = isA<string>('String', other) && other.includes(this.sample);
Comment on lines +260 to +261
Copy link
Contributor Author

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):

https://github.com/facebook/jest/blob/8759d63787b832af1fc9ac0ec2cc57b6f325ba9a/packages/expect/src/__tests__/asymmetricMatchers.test.ts#L321

At first it sounded strange, but reasons are very well explained in #7107


return this.inverse ? !result : result;
}
Expand All @@ -280,8 +280,8 @@ class StringMatching extends AsymmetricMatcher<RegExp> {
super(new RegExp(sample), inverse);
}

asymmetricMatch(other: string) {
const result = isA('String', other) && this.sample.test(other);
asymmetricMatch(other: unknown) {
const result = isA<string>('String', other) && this.sample.test(other);

return this.inverse ? !result : result;
}
Expand All @@ -297,6 +297,7 @@ class StringMatching extends AsymmetricMatcher<RegExp> {

class CloseTo extends AsymmetricMatcher<number> {
private precision: number;

constructor(sample: number, precision = 2, inverse = false) {
if (!isA('Number', sample)) {
throw new Error('Expected is not a Number');
Expand All @@ -311,8 +312,8 @@ class CloseTo extends AsymmetricMatcher<number> {
this.precision = precision;
}

asymmetricMatch(other: number) {
if (!isA('Number', other)) {
asymmetricMatch(other: unknown) {
if (!isA<number>('Number', other)) {
return false;
}
let result = false;
Expand Down
2 changes: 2 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2695,9 +2695,11 @@ __metadata:
version: 0.0.0-use.local
resolution: "@jest/expect-utils@workspace:packages/expect-utils"
dependencies:
"@tsd/typescript": ~4.8.2
immutable: ^4.0.0
jest-get-type: "workspace:^"
jest-matcher-utils: "workspace:^"
tsd-lite: ^0.6.0
languageName: unknown
linkType: soft

Expand Down