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

Prefer jest.MockedFunction<T> over jest.Mock #721

Open
entropitor opened this issue Nov 27, 2020 · 16 comments
Open

Prefer jest.MockedFunction<T> over jest.Mock #721

entropitor opened this issue Nov 27, 2020 · 16 comments
Labels

Comments

@entropitor
Copy link

In typescript

const fnMock = fn as jest.Mock;

is less ideal then

const fnMock = fn as jest.MockedFunction<
  typeof fn
>;

As the latter forces us to implement the mock correctly (have the right types), would a lint rule enforcing this behavior fall under the scope of this plugin?

@entropitor entropitor changed the title Prefer jest.MockedFunction<T> Prefer jest.MockedFunction<T> over jest.Mock Nov 27, 2020
@SimenB
Copy link
Member

SimenB commented Nov 27, 2020

I'm not sure if we should get involved with recommendations for @types/jest. Very much open to being convinced otherwise, tho.

@G-Rath @jeysal @thymikee thoughts?

@thymikee
Copy link
Member

I'd be more ok with it if the type was coming from Jest itself. Will pass on this one, won't pursue it, won't block it.

@SimenB
Copy link
Member

SimenB commented Nov 27, 2020

I'd be more ok with it if the type was coming from Jest itself.

Definitely. That's still some time out (unless someone else wants to pursue it), so might be worth doing something else in the meantime? Not sure

@jeysal
Copy link
Member

jeysal commented Nov 27, 2020

This would be the first typescript-eslint rule wouldn't it? I suppose it would need some other infra, a new preset to opt into or something?

@SimenB
Copy link
Member

SimenB commented Nov 27, 2020

Don't think so, it'd just look for some other nodes - if they're not there the rule does essentially nothing?

@jeysal
Copy link
Member

jeysal commented Nov 27, 2020

I guess in this case yeah, but some other TS-related rules may only be a good idea to activate when actually using TS, e.g. some sort of "force X to have a type declared", hence perhaps a ESLint-plugin/preset-jest-typescript may make sense

@G-Rath
Copy link
Collaborator

G-Rath commented Nov 28, 2020

I've thought about this in the past - I'm not against TS-related rules, but I couldn't come up with any that actually overlapped with jest that would be relevant to enough of the community to be worth shipping here.

Doing suggestions on types from @types/jest seemed like an obvious one, but the value didn't win out vs the effort in both implementation and maintenance vs. using @typescript-eslint/ban-types which is meant for exactly this sort of situation.

The math was around the range of types we could actually make recommendations for (as most of the types in @types/jest have at least one place where they're valid to use, which we couldn't test for), the quality of those recommendations (for similar reasons - types in test-land tend to become very nuanced, especially when using as), and accounting for the versioning of @types/jest, since everything is pretty much always added in a patch release so it'd be easy for a bunch of changes to be landed in a small space of time that makes our rule(s) some degree of wrong (this should be less of a problem in practice, as the types are pretty stable and you should always be able to update to the last, but it's still something that makes me a bit weary).

@G-Rath
Copy link
Collaborator

G-Rath commented Oct 9, 2021

Going to close this due to lack of activity

@G-Rath G-Rath closed this as completed Oct 9, 2021
@chtpl
Copy link

chtpl commented Mar 2, 2023

I would like to have such a rule as it might help against problems like the one described here: https://www.salto.io/blog-posts/typescript-unit-testing-pitfalls-with-jest-and-how-to-work-around-them#toc-type-safety-for-mocks

@SimenB
Copy link
Member

SimenB commented Mar 2, 2023

Since Jest now ships its own types, I think adding this rule is fine.

It should target types from @jest/globals tho, not @types/jest. If they overlap, great!

@SimenB SimenB reopened this Mar 2, 2023
@SimenB
Copy link
Member

SimenB commented Mar 2, 2023

Would you be able to provide a PR @chtpl?

@mrazauskas
Copy link
Contributor

mrazauskas commented Mar 2, 2023

The article is talking about @types/jest. I was scanning through it quickly. If I got it right, the problem is that by default jest.Mock returns (...args: Array<any>) => any. The default of jest.Mock which ships with @jest/globals is different: (...args: Array<unknown>) => unknown.

Other difference is that jest.Mock from @types/jest is complicated, because it requires passing the return and arguments type of a function. Usage of jest.MockedFunction is simply: jest.MockedFunction<() => string>. In @jest/globals both of them take just one type argument. Both are simple to use.

The difference between jest.Mock and jest.MockedFunction is very subtile (see documentation). I would recommended using jest.Mock to type the return type of jest.fn() (this is the case from the article).

jest.Mocked (and its constrained sisters: jest.MockedClass, jest.MockedFunction or jest.MockedObject) are meant to describe object (or member of that object) which is the result of calling jest.mock('some-module'). Importing from the mocked module, one will see that typings of mocks will be missing. These types are here to solve the issue.


What might be useful is a rule forcing to pass an argument to all of the utility types. So that the resulting type wouldn’t be based on unknown (that’s always the default), but on some concrete implementation. Here is are the types I have in mind:

  • jest.Mock
  • jest.Mocked, jest.MockedClass, jest.MockedFunction, jest.MockedObject
  • jest.Replaced
  • jest.Spied, jest.SpiedClass, jest.SpiedFunction, jest.SpiedGetter, jest.SpiedSetter

Most of the helper types were recently added. The issue is from the times when @types/jest had just two mock related types. jest.Mock was cumbersome to use because of multiple type arguments. At that time it made sense to recommend using jest.MockedFunction instead.

I was helping to add and polish these helper types to Jest repo (or @jest/globals). Also implemented them in @types/jest (as much as it was possible).

@chtpl
Copy link

chtpl commented Mar 2, 2023

@mrazauskas Thanks for the clarification. As I my journy in the JS/TS ecosystem just started, I don't understand what the difference between types/jest and jest/globals is and when I should use which of these two packages

@mrazauskas
Copy link
Contributor

See https://jestjs.io/docs/getting-started#type-definitions

For a new project I would go for import {describe, expect, test} from '@jest/globals';, but that is just an opinion.

@chtpl
Copy link

chtpl commented Mar 2, 2023

OK that clears it up a bit - so types/jest is a bit less verbose because I do not need to import describe, it, expect etc.
But why does jest/globals not define those functions the same way so that I wouldn't have to import them explicitly?

@mrazauskas
Copy link
Contributor

Importing from @jest/globals is the same as importing any other library. Install it and import the APIs. (By the way, any import is somewhat verbose. Why should I import what I just installed? JavaScript does not get that. Well.. TypeScript is build around the same idea.)

The @types/* libraries are special for TypeScript. They are allowed to declare typings of globals like process or window. So @types/jest goes this way and declares globals which Jest injects. For reference see https://www.typescriptlang.org/tsconfig#types

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants