search

dumbmatter / fakeIndexedDB

A pure JS in-memory implementation of the IndexedDB API
Apache License 2.0
562 stars 69 forks source link

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

Closed julienw closed 2 years ago

julienw commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 2 years ago

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