jakearchibald / idb

IndexedDB, but with promises
https://www.npmjs.com/package/idb
ISC License
6.4k stars 360 forks source link

4.0.0 #93

Closed jakearchibald closed 5 years ago

jakearchibald commented 5 years ago

@jeffposnick @philipwalton @inexorabletash as users of this library / IDB experts, I'd really appreciate a review of the readme for the new version.

Probably easier to look at https://github.com/jakearchibald/idb/blob/4.0.0/README.md than the diff.

quasicomputational commented 5 years ago

Road-testing the new version is running into a blocking problem for me: fake-indexeddb accesses cursor.source, but it's turning out to be undefined when the cursor's from async iteration. Here's a little reproducer:

import { openDB } from "idb/with-async-ittr.mjs";
import indexedDB from "fake-indexeddb";
import IDBKeyRange from 'fake-indexeddb/lib/FDBKeyRange';
import IDBIndex from 'fake-indexeddb/lib/FDBIndex';
import IDBCursor from 'fake-indexeddb/lib/FDBCursor';
import IDBDatabase from 'fake-indexeddb/lib/FDBDatabase';
import IDBTransaction from 'fake-indexeddb/lib/FDBTransaction';
import IDBObjectStore from 'fake-indexeddb/lib/FDBObjectStore';
import IDBRequest from 'fake-indexeddb/lib/FDBRequest';

global.indexedDB = indexedDB;
global.IDBKeyRange = IDBKeyRange;
global.IDBIndex = IDBIndex;
global.IDBTransaction = IDBTransaction;
global.IDBDatabase = IDBDatabase;
global.IDBCursor = IDBCursor;
global.IDBObjectStore = IDBObjectStore;
global.IDBRequest = IDBRequest;

const main = async () => {
    const db = await openDB("db", 1, {
        upgrade: (db) => {
            const foo = db.createObjectStore("foo", { autoIncrement: true });
            foo.add("hello");
            foo.add("goodbye");
        },
    });

    // Works:
    const tx = db.transaction(["foo"], "readwrite");
    const cursor = await tx.objectStore("foo").openCursor();
    cursor.delete();

    // Doesn't work:
    for await (const cursor of tx.objectStore("foo")) {
        cursor.delete();
    };
    await tx.done;
};

main();

Maybe something's not getting unwrapped properly somewhere?

jakearchibald commented 5 years ago

@quasicomputational thanks for spotting this. Reduced demo here https://static-misc.glitch.me/idb-lib-source-test/. I'll fix it.

jakearchibald commented 5 years ago

@quasicomputational fixed! Thanks for spotting.

quasicomputational commented 5 years ago

Looks good. I'm still getting one test failure, but it's a weird one and just the one - if there was a serious idb problem, I'd expect some of the related tests to be failing. I'll report back if I do track it down to idb, but thumbs up from me for the time being.

quasicomputational commented 5 years ago

I did just stub my toe on one thing: if, in one part of your code, you import from "idb" and another you import from "idb/with-async-ittr.js", the former can't unwrap the latter. The solution might be 'don't do that, then', but it's rather easy to do accidentally.

jakearchibald commented 5 years ago

@quasicomputational

you import from "idb" and another you import from "idb/with-async-ittr.js", the former can't unwrap the latter

I think that's a side effect of whatever build tool you're using. They should be the same, eg https://static-misc.glitch.me/idb-lib-unwrap-test/

quasicomputational commented 5 years ago

Oh, that makes some sense. I'm running it through Jest, using babel-jest; it wouldn't surprise me if there were some infelicities there.

I managed to pare down the bug I'm seeing and I can't reproduce it in a browser. Here's my current reproducer; it hangs after printing 'Done!' twice. So, yeah, probably not a bug in idb, but rather a race condition or similar in fake-indexeddb that's been newly exposed.

quasicomputational commented 5 years ago

Nope, probably an idb problem after all: an even more cut-down example shows that it's something to do with how transactions are wrapped, no promises needed. It still doesn't reproduce in the browser, though.

jakearchibald commented 5 years ago

@quasicomputational https://static-misc.glitch.me/idb-lib-source-test/ - I'm getting five logs for each example, as expected. What am I missing?

quasicomputational commented 5 years ago

I couldn't get it to happen in the browser either. But the two variants reliably do different things for me when invoked with node -r esm and using fake-indexeddb to mock IndexedDB for that environment.

jakearchibald commented 5 years ago

Feels like a fake-indexeddb bug.

jakearchibald commented 5 years ago

@jeffposnick thanks for the feedback. Lots of good catches there.

quasicomputational commented 5 years ago

Oh, I think you're right about it being a fake-indexeddb bug. Cutting down idb has revealed some weirdness going on with event listeners. I'll sort it out over there.

jakearchibald commented 5 years ago

Ok, let's ship this. Thank you all!