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(@jest/globals): add jest.Mocked, jest.MockedClass, jest.MockedFunction and jest.MockedObject utility types #12727

Merged
merged 21 commits into from
Aug 12, 2022
Merged

Conversation

mrazauskas
Copy link
Contributor

@mrazauskas mrazauskas commented Apr 24, 2022

From #12424 (comment)

Summary

This PR adds utility types jest.Mocked, jest.MockedClass, jest.MockedFunction and jest.MockedObject to the Jest object.

Test plan

Type tests are added.

@SimenB SimenB marked this pull request as draft April 24, 2022 21:31
@SimenB
Copy link
Member

SimenB commented Apr 24, 2022

exciting!

@SimenB
Copy link
Member

SimenB commented Aug 11, 2022

@mrazauskas hiya! what's the state here? 🙂 Would love to land it for 29!

@SimenB
Copy link
Member

SimenB commented Aug 12, 2022

This is shaping up! Do you think we should move changes in jest-mock to a separate PR, so this one is just exposing jest.Mocked etc?

@mrazauskas
Copy link
Contributor Author

It is a good question: if there is a need to expose jest.Mocked* helper types at all?

The only reason I see is that having jest.Mocked* makes it easier to migrate from @types/jest, but do we need them? The jest.Mocked<T> from this PR can be used instead of jest.MockedClass<T> and jest.MockedFunction<T>. It simply returns jest.MockedClass<T> or jest.MockedFunction<T> if class or function type is passed.

jest.Mocked<T> does the whole job. Other two will just take space in documentation (and will take time to write it too).

More over jest.mocked() does all what jest.Mocked<T> can do (see 715c564). Also migration to jest.mocked() is simple enough as we can see in #13117.

Perhaps all we need is jest.mocked()?


My only problem with jest.mocked() – shallow mocking by default. For instance, MockedClass must be deep. Otherwise MockSomeClass.prototype.methodA.mock.calls[0] does not work, because methodA is nested inside an object.

Also looking at jest.mocked(someFn, true) it is not clear what true does without reading docs. (By the way, jest.mocked(SomeClass) and jest.mocked(SomeClass, true) should be equivalent.)


What if we would skip adding jest.Mocked* helpers in favour of jest.mocked() and would swap the default of jest.mock() to deep mocking? Shallow mocking could be set though jest.mocked(someFn, {shallow: true})? Simple and readable? Or too much breaking?

@SimenB
Copy link
Member

SimenB commented Aug 12, 2022

It's useful if you use it as argument I think.

function helper(thing: jest.Mocked<SomeType>) {
  // ...
}

Right?

As for deep vs not deep - sounds like a separate PR from this?

@mrazauskas
Copy link
Contributor Author

Right. jest.Mocked<T> is useful. I just came up with one more use case. Let’s keep add it.

For me it makes sense to keep jest.Mocked<T>s implementation deep mocked. It was not before, in this PR implementation is deep. Does not sounds like jest.MockedDeep<T> should be introduced. If it is fine to keep it deep, then swapping defaults of jest.mocked() makes even more sense.

Alright. Let’s split this PR (;

@mrazauskas mrazauskas changed the title [wip] feat(@jest/globals, jest-mock): add jest.Mocked feat(@jest/globals): add jest.Mocked, jest.MockedClass, jest.MockedFunction and jest.MockedObject utility types Aug 12, 2022
@mrazauskas mrazauskas marked this pull request as ready for review August 12, 2022 15:05
@mrazauskas
Copy link
Contributor Author

@SimenB All done. Finally ;D

@SimenB
Copy link
Member

SimenB commented Aug 12, 2022

🎉

declare const jest: Jest;

// eslint-disable-next-line @typescript-eslint/no-namespace
declare namespace jest {
Copy link
Member

Choose a reason for hiding this comment

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

I love that we're finally adding type-only jest.* exports now! 👍

Hopefully we'll be able to replace most of @types/jest soonish with this approach

Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

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

Woo!

@SimenB SimenB merged commit 2f4340c into jestjs:main Aug 12, 2022
@mrazauskas mrazauskas deleted the feat-add-jestMocked branch August 13, 2022 04:31
@SimenB
Copy link
Member

SimenB commented Aug 19, 2022

@SimenB
Copy link
Member

SimenB commented Aug 25, 2022

@github-actions
Copy link

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.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants