search

invertase / react-native-notifee

Moved to https://github.com/invertase/notifee
https://invertase.io/blog/open-sourcing-notifee
Other
465 stars 31 forks source link

`jest-mock.js` has side effects #314

Closed Minishlink closed 3 years ago

Minishlink commented 3 years ago

Hello, unfortunately #299 breaks jest testing of apps relying on jest-mock.js. I feel like there should be two jest-mock file : one for internal testing of this library (if needed) and one user-facing. The one user facing shouldn't mock external libraries as it could have side effects for the user. For example, mocking EventEmitter with events does not really work in every case because RN returns an EventSubscription on addListener, whereas events does not. Likewise, I'm not sure jest.requireActual returns the previously mock if it was already mocked by the user, or the original module. If it returns the original module, react-native-notifee overrides the user's so that's not good too.

mikehardy commented 3 years ago

That's a good point. @helenaford we can (pretty easily?) include two jest setup files and split as mentioned - one "official external notifee jest mock file" and one "extra mocks needed for our testing"?

helenaford commented 3 years ago

@mikehardy yea I've actually been researching this week. When I updated the jest-mock file I didn't realise it was for external use. I'm going to update the README to Mark it as TODO.

helenaford commented 3 years ago

Fixed merged into master in #318, will be out in the next release.

helenaford commented 3 years ago

Thanks for your help @Minishlink. Update jest-mock.js has been updated and included in the latest release 1.8.1.