[jest] add type inference

[antoinebrault]

Updated jest typings to use latest typescript features to infer mock types.

Breaking Change

With types inferred, mockImplementation(), mockReturnValue(), mock...() will not compile if they are not used with the right types.

---

Please fill in this template.

• Use a meaningful title for the pull request. Include the name of the package modified.
• Test the change in your own code. (Compile and run.)
• Add or edit tests to reflect the change. (Run with npm test.)
• Follow the advice from the readme.
• Avoid common mistakes.
• Run npm run lint package-name (or tsc if no tslint.json is present).

Select one of these and delete the others:

If changing an existing definition:

• Provide a URL to documentation or source code which provides context for the suggested changes: private project
• Increase the version number in the header if appropriate.
• If you are making substantial changes, consider adding a tslint.json containing { "extends": "dtslint/dt.json" }.

[typescript-bot]

@antoinebrault Thank you for submitting this PR!

🔔 @JoshuaKGoldberg @tkrotoff @NoHomey @jwbay @asvetliakov @alexjoverm @epicallan @ikatyang @wsmd @JamieMason @douglasduteil @ahnpnl @JoshuaKGoldberg @UselessPickles @r3nya @Hotell @sebald @andys8 @dawnmist @geovanisouza92 @deadNightTiger @joscha @jonasheinrich @aldentaylor @bradleyayers - please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

If no reviewer appears after a week, a DefinitelyTyped maintainer will review the PR instead.

[jwbay]

Please restore the default type parameter and add one for Y. I don't think making these required is worth the breaking change, and trying to "force people" to use stronger types in tests is very opinionated. A large percentage of jest.Mock usages don't touch values, only call counts. Also keep in mind the inference for stronger mock types will only help with spyOn. Every other declaration will need to have the parameters passed explicitly, which is going to be a bit painful since Y is an arguments tuple that needs to be extracted.

[antoinebrault]

I know that is it very opinionated, but shouldn't we, as a community, enforce best practices?

I strongly believe that testing should be done the safest way possible, and "any" type should be avoided at all cost.

I don't agree that stronger mock types will only help for spyOn. There is a couple of example I changed in jest-tests (mockContextVoid, mockContextString, spy3Mock, spy4) where the typing of arguments would be lost and potentially mislead people.

If you don't care about tuple generics, you can still use <any, any[]> or without typing :

const spy = spyOn(obj, 'method');
const mock = jest.fn();
const mockContextVoid = jest.fn().mock;
const mock2 = spy.mockImplementation(() => null);

[nasreddineskandrani]

Please we want to move away from any when we are with typescript => so no default value with any

Arguments:

We need this contract for robust integrated tests (no any by default). I think this had to be corrected long ago (ts 2.8)
in other case the tests written are really weak.

This fix will put jasmine behind compared to jest and will raise the adoption of the library.

https://palantir.github.io/tslint/rules/no-any/

Using any as a type declaration nullifies the compile-time benefits of the type system.

---

Jest needs to guide devs to do it right and also produce quality tests. fallbacking on any is not at all going in this direction for the reasons posted on top.

If you plan to really go with this default value. I see one way to still force it by config:
It's to use the compiler flag noImplicitAny + the ts lint rule noAny.
This is a way to force it indirectly and keep default value any in jest types but i do not recommend this personnally.

@rickhanlonii can we have your opinion here, please?

[rickhanlonii]

A large percentage of jest.Mock usages don't touch values, only call counts

@jwbay can you explain this with an example of what you can do now and what this change would require?

[jwbay]

I know that is it very opinionated, but shouldn't we, as a community, enforce best practices?

To a point. We need to balance the benefits and costs considering the pain of breaking changes. As a data point, it's possible to make a package so annoyingly type-safe the change is reverted: #26813

I strongly believe that testing should be done the safest way possible, and "any" type should be avoided at all cost.

You are absolutely entitled to that strong opinion in your projects. The issue here is that this change impacts a large number of developers across the world, from enterprise teams to personal projects (this package gets 2 million downloads per week), for value that people will question. This change as-is isn't really just opinionated—it's prescriptive.

I don't agree that stronger mock types will only help for spyOn. There is a couple of example I changed in jest-tests (mockContextVoid, mockContextString, spy3Mock, spy4) where the typing of arguments would be lost and potentially mislead people.

That's fair, I didn't qualify my statement correctly. spyOn is the only way to infer types from existing functions. Everything else will either need explicit types, a runtime function to extract them via inference, or some kind of conditional type to convert typeof MyFunction to a correctly populated jest.Mock. We should consider adding that last option with this change, actually, assuming it goes forward. Looks like it's just T extends (...args: infer R) => infer V ? jest.Mock<V, R> : never

If you don't care about tuple generics, you can still use <any, any[]> or without typing :

That's part of the problem with this. The <any, any[]> is often just noise. Reading noise sucks and writing noise really sucks. Here are a couple examples:

test('foo', () => {
  someFunction();
  // type params are just noise here
  (foo as jest.Mock<any, any[]>).mockClear();
  someFunction();
  expect(fooMock).toHaveBeenCalledTimes(1);
});

test('foo', () => {
  // more noise
  (foo as jest.Mock<any, any[]>).mockName('MyMock');
  const rendered = render(<Component foo={foo} />);
  expect(rendered).toMatchSnapshot();
});

I think providing defaults for the cases where the type parameters truly don't matter is a reasonable ask. How about a compromise of defaulting to never? That way you can't interact with arguments or return values unsafely by default, and you can't supply an invalid implementation. It would still be a massive breaking change, but at least the types involved are being touched in some way, so it's more reasonable as a value proposition.

I'd really like to get the thoughts of other maintainers here considering the impact: @NoHomey, @asvetliakov, @alexjoverm, @epicallan, @ikatyang, @wsmd, @JamieMason, @douglasduteil, @ahnpnl, @JoshuaKGoldberg, @UselessPickles, @r3nya, @Hotell, @sebald, @andys8

As a note for impact, things like this are going to start popping up in codebases consuming this change:

// this function only touches one or two properties of a larger interface
mockFoo.mockReturnValue({
  x: 42
} as Partial<RealReturnType> as RealReturnType);

mockFoo.mockImplementation((a, _b, _c, _iReallyDontCare, _makeItStop) => {
  return Promise.resolve(a)
});

[antoinebrault]

updated description with breaking change

[JoshuaKGoldberg]

Thanks!

Although the rest of the changes look excellent (really looking forward to being able to strictly type spies/mocks!), that one point of requiring explicit type definitions on variables is not a good thing for many users. IMO we should default to any when not provided, not never.

Agreed that it's often better practice to explicitly type things, but not all users are interested in best practices, or the same best practices. Some users are just interested in having tests work out of the box, and any added pain given to them by TypeScript is a negative - even if it gives better type checking. A few examples:

• Large codebases converting from JavaScript would want as few places to need to explicitly type variables as possible.
• In a lot of folks' minds, unit tests don't need nearly the same level of type checking as source code, and if you explicitly fail/error within a few milliseconds, needing to write & update them constantly just gets in the way.

Speaking more generally: what are "best practices"? If your goals are to have completely strict type safety, then sure, enforcing explicit types helps enforce your personal best practices. But not everybody has that same goal. Some just want to write code quickly, be able to define a few non-object constructs such as interfaces, and get some improved level of IDE features. IMO we shouldn't sacrifice one use case (stricter typing) for others (quicker development, easier TS migration).

[nasreddineskandrani]

I understand all your arguments and i think it's case by case, team by team, project by project.

• Migration may require at the beginning an any default value.
• Experienced people may not need to force them do it right when at the opposite other teams need it.

So i am calling a crazy idea.
How to make this opinionated default value customizable for each of us depending on the needs.

^^
=> joke or no 🍡
a cutomizable way freestyle
toggle line 8 comment

Plan B: to keep any as default in jest types
Projects can use the compiler flag noImplicitAny + the ts lint rule noAny.
I'll try it during the weekend.

[nasreddineskandrani]

i did one test that show me a pure inconsistency
Example in image:

if the plan is to add only default value any for jest.Mock and jest.SpyInstance
ts error for the second test and the third one but not the first one => inconsistency
So i disagree with adding it back :) but i see that you insist on adding it back (hope you change your mind about it)

so i am fall backing to my plan b ^^ in the meantime

[nasreddineskandrani]

noImplicitAny didn't work with generik default any :/ (plan b => fail)

plan c:
I found a way by wrapping the type (based on @jwbay idea)
type CustomSpyInstance<A = never, B extends any[] = never[]> = jest.SpyInstance<A, B>; which override any/any[] and i ban explicit usage of jest.SpyInstance type from code base.

I use it as follow:
let spy: CustomSpyInstance<number, [Hero[]]>;
or
// this one fail in spyOn to force typing - prevent devs from passing any types
let spy: CustomSpyInstance;

this can be done at the project level not impacting all the devs around the world (migrating from js to ts ;) )

[typescript-bot]

@antoinebrault One or more reviewers has requested changes. Please address their comments. I'll be back once they sign off or you've pushed new commits or comments. Thank you!

[typescript-bot]

🔔 @jwbay - Thanks for your review of this PR! Can you please look at the new code and update your review status if appropriate?

[JoshuaKGoldberg]

Sorry, it's a little hard to parse through all the discussion and changes - what exactly are the breaking changes? Where are folks now being forced to use types when things defaulted to any before?

It sounds like they might be significant (for better or for worse), so even if they're the correct thing to do, there should be a clear summary of what's different. Jest is a popular enough library that any breaking change, no matter how small, is likely confusing and painful to some users.

[JoshuaKGoldberg]

-1 on the breaking change of jest.Mock and jest.SpyInstance needing to specify generics <return value, args type>.

+1 on everything else.

[antoinebrault]

Added back default values.
I still want the PR to be merged even thought I'm not in favor of keeping defaults.

@JoshuaKGoldberg

Large codebases converting from JavaScript would want as few places to need to explicitly type variables as possible.

What's the point of explicitly declaring jest.SpyInstance and jest.Mock if your not gonna use types. If people don't want to explicitly type variables in tests, they can still use implicit any. IDEs are still gonna autocomplete jest methods.

let spy; 
spy = spyOn(obj, 'method');
expect(spy).toHaveBeenCalled();

let foo;
test('foo', () => {
  foo.mockName('MyMock');
  const rendered = render(<Component foo={foo} />);
  expect(rendered).toMatchSnapshot();
});

In a lot of folks' minds, unit tests don't need nearly the same level of type checking as source code, and if you explicitly fail/error within a few milliseconds, needing to write & update them constantly just gets in the way.

This is wrong on so many levels. You want your tests to fail ASAP.
Once you add a new person on a project, the knowledge of that person will be limited. Tests should break when a contract changes (e.g. if an interface/method/property is updated).

Like @nasreddineskandrani said, there will be inconsistencies. Strong typing will be infered from fn(), spyOn(), Mocked<>, but SpyInstance, Mock declarations will not.

Anyway, for future reference, people can use a custom type and ban jest.SpyInstance/jestMock from their codebase if they want type safety.

[typescript-bot]

🔔 @jwbay @JoshuaKGoldberg - Thanks for your review of this PR! Can you please look at the new code and update your review status if appropriate?

[JoshuaKGoldberg]

Thanks for easing back on the breaking changes - I actually agree with your points on explicit typing being preferable, but I think we should discuss those breaking changes in a separate issue.

There's still one part I can find that's a breaking change here. This used to be allowed:

let spyUntyped: jest.SpyInstance;
spyUntyped = jest.spyOn(spiedTarget, "setValue");

...but now it gives a Type 'SpyInstance<void, [number]>' is not assignable to type 'SpyInstance<{}, any>'.

The only simple way to resolve this is to change SpyInstance to <T = any instead of <T = {}. That might itself be a breaking change (at the very least it's a little inaccurate, right) so maybe this isn't something that can be worked around...? Thoughts?

[antoinebrault]

@JoshuaKGoldberg you're right. I updated the code and added a test for that.

[JoshuaKGoldberg]

Awesome, thanks!

[nasreddineskandrani]

for your attention:
microsoft/TypeScript#28922

[Jessidhia]

Yeeeees, I've been yearning for this

The idea is great, but I think jest.fn can be made even easier to use by receiving a function type itself as the generic argument instead of separate args/return.

[Jessidhia]

You need Y extends never[], otherwise this refuses functions that have never parameters (rare, but possible).

declare function unreachable(arg: never): never

jest.fn(unreachable) // will error with `any[]`

[Jessidhia]

Actually, is there any reason why this is not just function fn<T extends (...args: never[]) => unknown>(implementation?: T): Mock<T>, with Mock changed to hold a function generic instead?

It makes declaring the Mock methods more annoying but it makes using .fn a lot easier when you want to make sure your mock function conforms to the interface of a function that already exists. Just call jest.fn<typeof myfunc>(/* inline function will even get contextual params/return types */).

[Jessidhia]

Also, the default type (when implementation is not specified) should be (...args: any[]) => void. Perhaps it should be on a 0 argument overload instead of an optional argument to avoid misuse.

[antoinebrault]

You need Y extends never[], otherwise this refuses functions that have never parameters (rare, but possible).

declare function unreachable(arg: never): never

jest.fn(unreachable) // will error with `any[]`

This is false. I just updated the tests and it doesn't error.

Actually, is there any reason why this is not just function fn<T extends (...args: never[]) => unknown>(implementation?: T): Mock<T>, with Mock changed to hold a function generic instead?

It makes declaring the Mock methods more annoying but it makes using .fn a lot easier when you want to make sure your mock function conforms to the interface of a function that already exists. Just call jest.fn<typeof myfunc>(/* inline function will even get contextual params/return types */).

Because this would be a breaking change. By adding a new generic with a default value, existing code will not be affected.

Also, the default type (when implementation is not specified) should be (...args: any[]) => void. Perhaps it should be on a 0 argument overload instead of an optional argument to avoid misuse.

It should not return void. This should compile:

jest.fn().mockImplementation((test: number) => test);

[Jessidhia]

Hmm... I need to brush up on the ...args: never[] thing, then. I had seen it as a recommendation from the TS team somewhere a few months ago.

As for the second one, that's because you're specifying a mock implementation after the .fn call but... that is supported API because JS is weird like that 😵

If you use .fn() without also adding a more specific .mock* it'll return void by default. But currently methods cannot alter the type of anything on their this...

[josecolella]

@antoinebrault Can you help me understand how to use the new:

function fn<T, Y extends any[]>(implementation: (...args: Y) => T): Mock<T, Y>;

I understand the T, but what is expected for the Y. After updating to this version, several of the mockImplementations, are throwing typing issues.

[antoinebrault]

@josecolella Y is the arguments types of the implementation function.

function x(y: number) {
  return y + '';
}
jest.fn(x) // will be Mock<string, [number]>

When mocking implementations, you have to use the same arguments types as the original function, but arguments are optional.

function x(y: number) {
  return y + '';
}
jest.fn(x).mockImplementation((y) => y * 2 + ''); // y is inferred to a number
jest.fn(x).mockImplementation(() => ''); // will work too

[josecolella]

@antoinebrault What about for mockImplementation of classes? As of now it's complaining that I need to implement all the methods within the class, when in reality only one method needs to be mocked.

interface SomeInterface {
      foo() 
      foo2()
}

jest.fn<SomeInterface>(() => ({
 foo: jest.fn().mockReturnValue(true)
}));

With new types this no longer works. Do you have an example of how to use this new version with class mock implementations?

[antoinebrault]

@antoinebrault What about for mockImplementation of classes? As of now it's complaining that I need to implement all the methods within the class, when in reality only one method needs to be mocked.

you can use Partial for that use case. jest.Mocked<Partial<Type>>

interface SomeInterface {
      foo() 
      foo2()
}

jest.fn<SomeInterface>(() => ({
 foo: jest.fn().mockReturnValue(true)
}));

With new types this no longer works. Do you have an example of how to use this new version with class mock implementations?

interface SomeInterface {
    foo(): string,
    foo2(): number
}
const y: jest.Mocked<SomeInterface> = {
    foo: jest.fn().mockReturnValue(''),
    foo2: jest.fn().mockReturnValue(1)
};

[josecolella]

@antoinebrault Thanks for your explanation and examples.

[typescript-bot]

@antoinebrault I haven't seen anything from you in a while and this PR currently has problems that prevent it from being merged. The PR will be closed tomorrow if there aren't new commits to fix the issues.

[typescript-bot]

@antoinebrault To keep things tidy, we have to close PRs that aren't mergeable but don't have activity from their author. No worries, though - please open a new PR if you'd like to continue with this change. Thank you!