Indexed DB + Promises #3

@ Indexed DB Promises.md

Moved to: https://github.com/inexorabletash/indexeddb-promises

... to support more fine-grained commenting and collaboration.

Thoughts while reading

• Having DOMErrors still exist in the API gets more painful with promises, which really should not be rejected with DOMErrors (only with DOMExceptions).
• I think instead of a promise-returning .promise() method that re-generates a new promise every time, it would make more sense to have a .promise getter with a cached promise. This also avoids re-creating the relevant promise on every .then() or .catch() call.
• "Methods that return requests still throw rather than reject on invalid input, so you must still use try/catch blocks" this really sucks, and argues for removing .then() and .catch() since they are misleading now.

Thanks!

• Replaced DOMError with DOMException (I've made the change to the IDB v2 spec as well, optimistically)
• Made promise an attribute, removed then() and catch()
• Still need to noodle on returning cached promise vs. new one each time. cached is probably fine, but polyfill doesn't reflect that.

Updated the polyfill to return same promise each time (and asserts in the example)

This is great. Some thoughts:

transaction.promise - should this be transaction.complete? As in await transaction.complete

https://code.google.com/p/chromium/issues/detail?id=457409 makes me sad, I wish transactions unset "active" in their own microtask, which comes after microtasks queued in a listener. Having to use waitUntil for store.get('hello').then(val => store.put(val, 'foo')) feels a bit much.

request.promise - could IDBRequest become a thenable with a @species of Promise? Hah, am I arguing for something that @domenic argued against?

transaction.complete makes sense (when I started off with .then and .catch I wanted symmetry...)

Re: active flags vs. microtasks - see w3c/IndexedDB#27 (for other readers)

Re: Making IDBRequest thenable - I'd rather some other spec be the first penguin. :)

Hrm... should the promise getter be called transaction.finished since the transaction could 'complete' (succeed) or 'abort' (fail)? Or is transaction.complete fine because rejection is clear enough?

I think .complete as it fits with the equivalent event. A transaction can finish with a failure or abort, but you'd want a promise that indicated success & would reject if unsuccessful.

I'm really excited to see this - as I've just hit all these problems trying to get IDB to work with async await. I agree with Domenic about DOMError, etc. thanks for already addressing that! + also the other stuff you've already addressed. I also agree with Jake about transaction.complete and about .waitUntil being too much for .get().

Can I urge you to please write your examples and trials in terms of async await? .then()-using code is going to increasingly become an unnecessary code smell.

The promise attribute is a convenience to allow IDBTransaction objects to be used in Promise chains. It is roughly equivalent to:

Promise properties should vend the same promise each time. So tx.promise === tx.promise.

await tx;

Guess this should be await tx.promise;

An explicit abort() call still also aborts the transaction immediately, and the promise resolution is ignored.

Will this cause the promise returned by waitUntil to reject? Hah, just saw your note on this. In ServiceWorker waitUntil returns void. It also throws if the state is invalid.

ISSUE: Add a timeout to transactions. Maybe make this mandatory if waitUntil() is used?

This is really tricky. As a developer, how would I choose a timeout given my code will be running on devices with different cpus etc? Would timeout be a simple timer, or related to transaction inactivity?

await tx.complete; // Ensure it commits

I think I prefer .complete, but earlier you call it .promise.

The cursor iteration methods (continue() and advance()) now return Promise<IDBCursor?>. NB: Previously they were void methods, so this is backwards-compatible.

Unfortunately they can throw errors, which would violate http://www.w3.org/2001/tag/doc/promises-guide#always-return-promises if they returned promises. The answer here is either willful violation (I imagine @domenic would be against this, and it would be messy in terms of WebIDL), or the cursor returned by .openCursor().promise is different to the cursor in idbResult.value, in that it has some kind of flag set that makes .advance() return a promise rather than void.

Here's how you'd fetch all keys in a range using a cursor:

FWIW, here's the ES6 version:

function getAll(store, query) {
  let result = [];
  return store.openCursor(query).promise.then(function ittr(cursor) {
    if (!cursor) return result;
    result.push(cursor.value);
    return cursor.continue().then(ittr);
  });
}

let cursor = await store.openCursor(query);

Needs .promise at the end?

I still think I'd like IDBRequest and IDBTransaction to have then and catch methods that proxy to an underlying promise. Seems like forgetting to add .promise is going to be common.

@domenic I guess this idea makes you sick in your mouth?

Thanks, @jakearchibald !

Promise properties should vend the same promise each time. So tx.promise === tx.promise.

Fixed in the "roughly equivalent to" example. Already present in the polyfill and normative text.

Guess this should be await tx.promise;

Yes, I missed a few of those. Bleah. Added 'em in.

I think I prefer .complete, but earlier you call it .promise.

I'd switched over but forgot when I made some edits. Corrected - it's .complete everywhere.

Unfortunately they can throw errors, which would violate http://www.w3.org/2001/tag/doc/promises-guide#always-return-promises if they returned promises

Bleah, good catch. Will ponder.

One option for IDBCursor's advance()/continue() would be to have them return an IDBRequest which then has .promise hanging off of it. The question is: what IDBRequest? Those methods currently reset the readyState of the original IDBRequest used to open the cursor from "done" back to "pending" and fire off new "success" or "error" events.

How about

• cursor() and advance() return the same IDBRequest originally used to open the cursor (NOTE: I've wanted this in other polyfills!)
• Generate a new internal, unfulfilled Promise (the one returned by .promise) for the request at the same time as the readyState is reset

So rq.promise === rq.promise would still hold, just not over time. Iteration would then look like:

async function getAll(store, query) {
  let result = [];
  let cursor = await store.openCursor(query).promise;
  while (cursor) {
    result.push(cursor.value);
    cursor = await cursor.continue().promise; // only change
  }
  return result;
}