indexeddb-promises icon indicating copy to clipboard operation
indexeddb-promises copied to clipboard

Revisit then() / catch() on IDBRequest

Open inexorabletash opened this issue 9 years ago • 4 comments

Over on https://gist.github.com/inexorabletash/8c122c84a65de65c35b3 @domenic objected to including then() and catch() helpers on IDBRequest.

The pro of having those is that IDBRequest becomes a thennable which makes usage cleaner in promise chains and with await.

The con is that methods returning IDBRequest still throw on invalid input, which is icky.

After removing them I've had multiple requests to add them back, so adding this bug to gather feedback. A few options:

  • Add them now
  • Never add them (never say never...)
  • Plan to add them later, once async/await adoption has grown, since within an async the issue goes away.

FWIW, they could be shimmed with:

IDBRequest.prototype.then = function() { var p = this.ready; return p.then.apply(p, arguments); };
IDBRequest.prototype.catch = function() { var p = this.ready; return p.catch.apply(p, arguments); };

(And ditto for all of the above w/ IDBTransaction.)

inexorabletash avatar Dec 09 '15 22:12 inexorabletash

What about changing the methods returning IDBRequest to not throw on invalid input?

domenic avatar Dec 12 '15 01:12 domenic

Per offline conversations with @domenic it seems like we should try making requests thennables.

I don't think we can change the methods to not throw; much existing code relies on this, including libraries that wrap methods with promises.

inexorabletash avatar Jun 20 '16 18:06 inexorabletash

Presumably the IDL would look like:

interface IDBThennableThing {
  ...
  Promise<any> then(any onFulfilled, optional any onRejected);
  Promise<any> catch(any onRejected);
  ...
};

I also assume this is rare enough that we wouldn't want thennable<> in WebIDL or anything funky like that.

inexorabletash avatar Jun 20 '16 23:06 inexorabletash

Noted one possible issue with this:

https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/storage/indexeddb/resources/promise-idb.js?l=15

This is an existing bit of code that returns a transaction via a Promise:

  const transact = (db, scope, mode) => Promise.resolve(db.transaction(scope, mode));

This is used in a test file to kick off a promise chain. The semantics would change if transaction becomes a thennable.

  • Current behavior: resolve immediately with a transaction ready to issue requests against
  • New behavior: wait for the transaction to commit, then resolve with the finished transaction.

We need to investigate other IDB wrappers and see if this would break them.

inexorabletash avatar Apr 14 '17 00:04 inexorabletash