Closed dumbmatter closed 3 years ago
Let me think about this. Seems reasonable though.
I wonder if it would be easier to polyfill getAll in this library via an optional import (like I currently do with async iterators)
The npm download stats of my polyfill suggest that either people don't care about old browser support or they don't use getAll. So you shouldn't lose much sleep over it. I guess a similar argument could have prevented me from opening this issue in the first place :)
Meh, it's difficult to judge these things by npm stats.
Fwiw, version 3 of this library did polyfill getAll https://github.com/jakearchibald/idb/blob/e1c7c44dbba38415745afc782b8e247da8c833f2/lib/idb.mjs#L259, but I dropped it in a rewrite. I think you're the first to create an issue about it, so yeah, maybe it isn't a big deal.
On the other hand, it would be pretty easy to add the polyfill back as an optional extra.
Hey there, @jakearchibald and @dumbmatter.
It's nice to find this discussion here, because I'm actually a user of idb
and the lack of support for Edge was a sad realization for us. In order to solve our app crashing on older Edge versions I actually implemented a polyfill based on @dumbmatter 's mentioned library.
Just to contribute to the discussion, an idb
pollyfill for the getAll
method for unsupported browsers, or at least the tiny change that @dumbmatter suggested in their initial comment, would be very much desirable.
Either way, thanks a lot to both of you for the work on these libraries.
Have a look at how additional traps are added to the library https://github.com/jakearchibald/idb/blob/master/src/database-extras.ts#L55.
You could use this to set a trap for addAll
.
I'd also appreciate adding the polyfill back and in.
Can we mark this issue as Edge relevant?
Also judging by the polyfill that we employed naively it's rather easy to employ either in core or in docs, but not easy to catch / debug (in Edge) / understand / or do as developer. Edge is relevant enough for that imo, still.
Kind regards
Since the number of Edge users is decreasing, I've decided not to implement this. However, here's a polyfill for anyone that needs it:
import type { StoreNames, IDBPDatabase, StoreValue } from "idb";
async function getAll<
DBTypes,
StoreName extends StoreNames<DBTypes> = StoreNames<DBTypes>
>(
db: IDBPDatabase<DBTypes>,
storeName: StoreName
): Promise<StoreValue<DBTypes, StoreName>[]> {
if (db.getAll) return db.getAll(storeName);
const items: StoreValue<DBTypes, StoreName>[] = [];
let cursor = await db.transaction(storeName).store.openCursor();
while (cursor) {
items.push(cursor.value);
cursor = await cursor.continue();
}
return items;
}
Demo: https://codesandbox.io/s/awesome-faraday-9q6bz?file=/src/index.ts
Plain JS version:
async function getAll(db, storeName) {
if (db.getAll) return db.getAll(storeName);
const items = [];
let cursor = await db.transaction(storeName).store.openCursor();
while (cursor) {
items.push(cursor.value);
cursor = await cursor.continue();
}
return items;
}
Although all modern browsers support getAll, old versions don't. A while back I wrote indexeddb-getall-shim, and I'm still using it some places because I still have users on those old browsers.
Problem is, idb does not like my polyfill. Specifically it breaks on this line:
I don't think it is possible for me to create an actual IDBRequest, so instead I created a fake one. But I can't make
myFakeRequest instanceof IDBRequest
return true (well, I can by setting its prototype to IDBRequest, but it seems this results in browsers restricting what I can do with the object, making it useless for my purposes).If instead your code was changed to:
then my polyfill would work fine.
Are you interested in adding this ugliness to your library? If so, I'm happy to make a PR.