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

Global jest type from test-utils causing issues when existing test files also interact with the global jest object #710

Closed
philwolstenholme opened this issue Dec 27, 2024 · 3 comments · Fixed by #712
Labels

Comments

@philwolstenholme
Copy link

Hi, thanks for your work on this package :)

I just used patch-package to patch react-intersection-observer@9.14.0 for the project I'm working on. I don't think this is the best fix, but it resolved an issue I'd been having.

In some of our existing test files we have to do a little trick like this:

// https://github.com/testing-library/react-testing-library/issues/1197#issuecomment-1693824628
// @ts-expect-error type hoohaa
globalThis.jest = {
  advanceTimersByTime: vi.advanceTimersByTime.bind(vi)
};

You can check out the RTL issue link for more context of why we have to do this. There may be a better way, but this has worked for our project for almost a year without any issues.

I added react-intersection-observer to our project today and used react-intersection-observer/test-utils. We then started getting TypeScript errors like these:

src/app/(shopfront)/(simplified)/checkout/payment/complete/CompletePage.test.tsx:71:5 - error TS2353: Object literal may only specify known properties, and 'advanceTimersByTime' does not exist in type '{ fn: <T extends Procedure = Procedure>(implementation?: T | undefined) => Mock<T>; }'.

71     advanceTimersByTime: vi.advanceTimersByTime.bind(vi)
       ~~~~~~~~~~~~~~~~~~~

The global jest type from react-intersection-observer/test-utils was affecting our test files and resulting in errors.

I was wondering if the type from react-intersection-observer/test-utils needs to be global? If that type is just for your own development needs then a module-scope type might be better, I think?


Here is the diff that solved my problem:

diff --git a/node_modules/react-intersection-observer/test-utils/index.d.mts b/node_modules/react-intersection-observer/test-utils/index.d.mts
index 58a5d85..464972a 100644
--- a/node_modules/react-intersection-observer/test-utils/index.d.mts
+++ b/node_modules/react-intersection-observer/test-utils/index.d.mts
@@ -1,9 +1,3 @@
-declare global {
-    var IS_REACT_ACT_ENVIRONMENT: boolean;
-    var jest: {
-        fn: typeof vi.fn;
-    } | undefined;
-}
 /**
  * Create a custom IntersectionObserver mock, allowing us to intercept the `observe` and `unobserve` calls.
  * We keep track of the elements being observed, so when `mockAllIsIntersecting` is triggered it will
diff --git a/node_modules/react-intersection-observer/test-utils/index.d.ts b/node_modules/react-intersection-observer/test-utils/index.d.ts
index 58a5d85..464972a 100644
--- a/node_modules/react-intersection-observer/test-utils/index.d.ts
+++ b/node_modules/react-intersection-observer/test-utils/index.d.ts
@@ -1,9 +1,3 @@
-declare global {
-    var IS_REACT_ACT_ENVIRONMENT: boolean;
-    var jest: {
-        fn: typeof vi.fn;
-    } | undefined;
-}
 /**
  * Create a custom IntersectionObserver mock, allowing us to intercept the `observe` and `unobserve` calls.
  * We keep track of the elements being observed, so when `mockAllIsIntersecting` is triggered it will

This issue body was partially generated by patch-package.

@thebuilder
Copy link
Owner

thebuilder commented Dec 29, 2024

This makes sense - I guess we might just have ignore types jest, so we don't get it mixed up. The variables are set globally by Jest/Testing Library. Not sure if there's another way of defining global variables scoped to a module?

@thebuilder
Copy link
Owner

🎉 This issue has been resolved in version 9.14.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@philwolstenholme
Copy link
Author

Thanks for the new releaese @thebuilder, it solves both my issues!

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

Successfully merging a pull request may close this issue.

2 participants