Replace usage of Jest globals with @jest/globals package

marchaos / jest-mock-extended

Type safe mocking extensions for Jest https://www.npmjs.com/package/jest-mock-extended

MIT License

839 stars 57 forks source link

Replace usage of Jest globals with @jest/globals package #119

Closed macko911 closed 4 weeks ago

macko911 commented 1 year ago

Attempts to fix https://github.com/marchaos/jest-mock-extended/issues/116

Removed @types/jest package.

Added @jest/globals as peer dependency. I'm not sure about which versions to support, but Jest 28 introduced a breaking change in jest.Mock type definition so I'm not confident how it would work with earlier versions.

Note that this definition is different between @types/jest@29 and @jest/globals@29

The change is described in https://github.com/jestjs/jest/issues/12479 tl;dr the generic type is changed from jest.fn<ReturnType<MyFunctionType>, Parameters<MyFunctionType>> to jest.fn<MyFunctionType>

I did my best to fix the new type errors in the library but I had to add a couple of // @ts-expect-error comments in the process. The new types work across internal tests and when used as a dependency in our own repo but I'm not sure if they are valid for every existing use case for this library.

zavarka commented 10 months ago

Hi, @skovhus. Thanks for reviewing this merge request. Can you please merge it? I'm hitting this issue too and would love to get this fix. Thank you.

And thank you, @macko911, for putting it together!

skovhus commented 10 months ago

Hi, @skovhus. Thanks for reviewing this merge request. Can you please merge it? I'm hitting this issue too and would love to get this fix. Thank you.

I would love to, but I'm not a maintainer of jest-mock-extended. ;)

FYI @marchaos

leighman commented 8 months ago

I think this would also help with bun compatibility since it rewrites references to @jest/globals ? It doesn't seem to work currently

darthmaim commented 1 month ago

Hey @marchaos, can we somehow help to move this PR along? Is there something still missing for you?

marchaos commented 4 weeks ago

Hey, I'll take a look at this now.

marchaos commented 4 weeks ago

I've pushed some changes:

Fixed the conflicts with master
Removed the ts-expect-error on the tests which was a genuine TS issue.
A couple of chore tasks - changed ts-ignore to ts-expect error and removed those that weren't ignoring anything, ugraded ts-essentials.

The changed to CalledWithMock to take 1 argument instead of 2 is a breaking change as this type is exported and may cause TS errors for anyone upgrading. It's not something that is used often though.

I ran this branch against a private code base that uses jest-mock-extended extensively, and all worked as expected once I fixed the CalledWithMock, except one place which resulted in a TS error, which seems a bit odd.

Would be good for someone to take a look at my changes, and potentially try this branch out for themselves? I can release a beta (4.0.0-beta) to npm of that makes it easier?

darthmaim commented 4 weeks ago

I've just tested this branch in one of my projects (which admittedly only uses jest-mock-extended very sparingly), and everything was working nicely!

marchaos commented 4 weeks ago

I've pushed a version to npm 4.0.0-beta1 if you want to try this out. Will release 4.0.0 with these fixes ones we've garnered some feedback.

leighman commented 3 weeks ago

Gave it a try.

Had an additional type error

Type 'DeepMockProxy<SigningKey>' is not assignable to type 'void | Promise<SigningKey>'

but I think it's actually correct/the problem is with the other types.

Otherwise seems good.