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.

Sign up for GitHub

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

chore: followup to jest-mock TS migration #7850

Merged
merged 4 commits into from
Feb 10, 2019
Merged

chore: followup to jest-mock TS migration #7850

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d8302b8
chore: followup to jest-mock TS migration
thymikee Feb 10, 2019
47f5522
update changelog
thymikee Feb 10, 2019
c12feb3
moar
SimenB Feb 10, 2019
caa0257
simplify
thymikee Feb 10, 2019
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
Prev Previous commit
Next Next commit
moar
  • Loading branch information
@SimenB
SimenB committed Feb 10, 2019
commit c12feb3bc2c8cbd24455c97df4c6d48c9e262643
6 changes: 3 additions & 3 deletions packages/jest-mock/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -911,7 +911,7 @@ class ModuleMockerClass {
return metadata;
}

isMockFunction(fn: any): boolean {
isMockFunction<T, Y extends unknown[]>(fn: any): fn is Mock<T, Y> {
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if this makes sense or not

Copy link
Collaborator Author

@thymikee thymikee Feb 10, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Y is optional and unknown[] by default, so you can omit that. Docs say that type gruard performs runtime check, but I don't see a change in the build output. Anyway, it makes sense to me

Copy link
Member

Choose a reason for hiding this comment

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

the built output is the same, this is just us telling the type system that if the function returns true what the type is. Basically a typeof

return !!fn && fn._isMockFunction === true;
}

Expand Down Expand Up @@ -943,7 +943,7 @@ class ModuleMockerClass {
? SpyInstance<ReturnType<T[M]>, ArgsType<T[M]>>
: never;

spyOn<T extends {}, M extends keyof T>(
spyOn<T extends {}, M extends NonFunctionPropertyNames<T>>(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

heh, not this is not entirely true because this generic spyOn can accept function and non-functional property names, but TS seems to buy it.

I don't really understand why a function even needs type definitions when it's overloaded like this. Generated types are only for the definitions as well, so that seems really counterintuitive, not to mention it causes headaches on "how to fit all the typings in this def and make TS happy, even though it may be a lie" 😅

Copy link
Member

Choose a reason for hiding this comment

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

heh, yeah. we might want to add actual tests on the types at some point (e.g. https://www.npmjs.com/package/tsd-check or https://www.npmjs.com/package/dts-jest)

object: T,
methodName: M,
accessType?: 'get' | 'set',
Expand Down Expand Up @@ -985,7 +985,7 @@ class ModuleMockerClass {
return object[methodName];
}

private _spyOnProperty<T extends {}, M extends keyof T>(
private _spyOnProperty<T extends {}, M extends NonFunctionPropertyNames<T>>(
obj: T,
propertyName: M,
accessType: 'get' | 'set' = 'get',
Expand Down