chore: followup to jest-mock TS migration #7850
Conversation
packages/jest-mock/src/index.ts
Outdated
| @@ -973,7 +985,7 @@ class ModuleMockerClass { | |||
| return object[methodName]; | |||
| } | |||
|
|
|||
| _spyOnProperty<T extends {}, M extends keyof T>( | |||
| private _spyOnProperty<T extends {}, M extends keyof T>( | |||
There was a problem hiding this comment.
| private _spyOnProperty<T extends {}, M extends keyof T>( | |
| private _spyOnProperty<T extends {}, M extends NonFunctionPropertyNames<T>>( |
?
There was a problem hiding this comment.
This causes TS to choke :( Wanna have a look locally?
packages/jest-mock/src/index.ts
Outdated
| @@ -899,7 +911,7 @@ class ModuleMockerClass { | |||
| return metadata; | |||
| } | |||
|
|
|||
| isMockFunction(fn: any): boolean { | |||
| isMockFunction<T, Y extends unknown[]>(fn: any): fn is Mock<T, Y> { | |||
There was a problem hiding this comment.
I'm not sure if this makes sense or not
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
| object: T, | ||
| methodName: M, | ||
| ): T[M] extends (...args: any[]) => any | ||
| ? SpyInstance<ReturnType<T[M]>, ArgsType<T[M]>> | ||
| : never; | ||
|
|
||
| spyOn<T extends {}, M extends keyof T>( | ||
| spyOn<T extends {}, M extends NonFunctionPropertyNames<T>>( |
There was a problem hiding this comment.
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" 😅
There was a problem hiding this comment.
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)
|
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
Produces better public interface to the module.
See: #7847 (comment) and DefinitelyTyped/DefinitelyTyped#32882
Test plan
CI happy