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

Issues with latest version of jest with the jsdom environment #64

Closed
julienw opened this issue Jun 7, 2021 · 3 comments
Closed

Issues with latest version of jest with the jsdom environment #64

julienw opened this issue Jun 7, 2021 · 3 comments

Comments

@julienw
Copy link

julienw commented Jun 7, 2021

Hey,

Recently, the jest folks removed setImmediate and clearImmediate from the jsdom environment (they had it before incorrectly). This is a good thing.
fakeIndexedDB uses the setimmediate polyfill in case setImmediate isn't present. This is all good.

But... there's a problem when we start using the fake timers. Indeed, the polyfill seems to rely on setTimeout (although I'm not 100% sure why, from their code it should be process.nextTick... which isn't a macrotask either in recent node versions actually), and setTimeout is using the fake timeouts from jest so never ends.

The testing-library folks try to detect if we're in jest with fake timers, and in that case they run some code using the real timers:
https://github.com/testing-library/dom-testing-library/blob/c273ed518cf783591692be8c0c5e5a34feafd4bb/src/helpers.js#L7-L12
Could that be a solution for fakeIndexedDB? Should you move to process.nextTick anyway?

Thanks for this wonderful tool!

@dumbmatter
Copy link
Owner

dumbmatter commented Jun 20, 2021

Switching from setImmediate to process.nexTick does make a few of the tests fail. Not sure why, I haven't had a chance to look into it further. But IIRC it was difficult to get all of the asynchronous stuff working correctly, and I think even now there are some rare edge cases where it doesn't work quite right. Very finicky stuff. So I'm a little scared to change it :)

@julienw
Copy link
Author

julienw commented Jul 5, 2021

I tried using queueMicrotask but got a few timeout in tests (I guess the same ones you got with process.nextTick). I think the difference is that setImmediate is a macrotask, but process.nextTick and queueMicrotask queue microtasks, which behave differently.

I also tried using setTimeout and with this, all tests are passing. The downside of setTimeout is that in browsers it may be delayed more, especially when nested. But is that a problem for this package, as it will run in a test environment? That would remove the need of the polyfill for setImmediate, reducing the complexity.

But anyway this doesn't solve the problem with jest's fake timers, because they also mock setTimeout obviously. Instead of calling setImmediate or setTimeout, we'd need to call the real functions, probably using some magic with jest functions like outlined above.

For now I'm not blocked anymore in our project, as I removed the fake timers from the tests that were using this library.

@dumbmatter
Copy link
Owner

Hopefully this is fixed in v3.1.4. Please reopen if not.

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

Successfully merging a pull request may close this issue.

2 participants