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

Transactions/Cursors fail with jsdom #67

Closed
ph-fritsche opened this issue Oct 11, 2021 · 7 comments
Closed

Transactions/Cursors fail with jsdom #67

ph-fritsche opened this issue Oct 11, 2021 · 7 comments

Comments

@ph-fritsche
Copy link

With jsdom environment in jest@27 acting on a transaction after allowing other code to run (per await) throws errors.
This differs from executing the same calls in a node environment.

Maybe this is related to #64

https://github.com/ph-fritsche/repro-jest-jsdom-indexeddb

@dumbmatter
Copy link
Owner

Thanks for the repo with a reproduction!

It does seem like it could be related to #64... but if I put jest.useRealTimers(); at the top of your test script, it still fails. So maybe it's something else jsdom is doing, not the timers? I dunno. I've never really used it before.

Anyone have any ideas? If it was as simple as using real timers, I could do it like suggested in #64 probably, but it doesn't seem to be that simple...

@ph-fritsche
Copy link
Author

The problem seems to be here:

this._rawDatabase.processTransactions(); // See if can start right away (async)

This starts and completes the transaction per setImmediate.
But setImmediate executes the callback before process.nextTick in jsdom.

const tx = db.transaction('foo', 'readwrite')
await new Promise(r => process.nextTick(r))
// with node environment: { _state: 'active' }
// with jsdom environment { _state: 'finished' }

I've confirmed the following workaround in the repro:

global.setImmediate = cb => setTimeout(cb, 0)

Would it be feasible to use setTimeout in the the Database.processTransactions() implementation?

@dumbmatter
Copy link
Owner

In general it is difficult to switch between the timing functions as described in #64. It may be possible in this specific case, I'm not sure, but before I even get there... it doesn't seem to help here. You are correct that global.setImmediate = cb => setTimeout(cb, 0) at the top of your test file makes it work. But changing the setImmediate call to setTimeout in repro-jest-jsdom-indexeddb/node_modules/fake-indexeddb/build/lib/Database.js does not get rid of the error. Not sure why, I guess the timer replacement is doing something sophisticated that I don't understand.

Like... any idea about why jest.useRealTimers(); doesn't seem to help? Is jest/jsdom still messing with the timers even after that?

@ph-fritsche
Copy link
Author

There are no fake timers in place. jest.useRealTimers() has no effect.
(And I think if you use fake timers, you would just have to advance them.)

Replacing setImmediate with setTimeout in build/lib/Database fixes test "transaction is active on next tick".
Also replacing setImmediate in build/FDBTransaction then fixes the initial two tests.

The clean solution might be to configure the timer to be used per setup.

import { setNextMacroTask } from 'fake-indexeddb/auto'
setNextMacroTask(setTimeout)

@ath0mas
Copy link

ath0mas commented Oct 14, 2021

After update from v3.1.3 to v3.1.4 that contains only this fix, my Mocha tests started failing 😢
Error: Timeout of 2000ms exceeded. ...
I'll try to create a reproduction repo before opening corresponding issue

@ph-fritsche
Copy link
Author

@ath0mas Do you have fake timers in place that might need to be advanced so that the calls to setTimeout in 17c1547 are resolved?

@joshkel
Copy link
Contributor

joshkel commented Oct 18, 2021

@ath0mas I'm running into some test timeout issues too; I opened #69 with my findings so far.

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.

4 participants