Skip to content
This repository has been archived by the owner on Feb 13, 2021. It is now read-only.

Consider async iterables for cursors #8

Open
ghost opened this issue May 8, 2016 · 8 comments
Open

Consider async iterables for cursors #8

ghost opened this issue May 8, 2016 · 8 comments

Comments

@ghost
Copy link

ghost commented May 8, 2016

Hi! I'm wondering if it makes sense to turn IDBCursor into an async iterable, once that proposal crystallizes a bit. The getAll example in the readme is already quite nice, and it seems like the proposed for await syntax is a natural way to iterate over a result set.

If this was discussed before, apologies for the dupe. I first thought streams might be a good fit, but the streams API FAQ pointed me to async iterables, and that got me thinking about how to make them work with IDB.

Thanks!

@inexorabletash
Copy link
Owner

Agred - once that settles, it makes sense to pursue.

@inexorabletash
Copy link
Owner

inexorabletash commented Jul 22, 2016

Some initial thoughts here. I need to vet this by @littledan

Handwavy Proposal #1

Assumptions:

Here's how you'd write a basic adapter to make cursors async-iterable:

async function* openCursor(source, range) {
  let cursor = await source.openCursor(range).ready;
  while (cursor) {
    yield cursor;
    cursor = await cursor.continue().ready;
  }
}

And you'd use it like:

for await (let c of openCursor(store, range)) {
  console.log(c.key, c.value);
}

So far, so good?


Now if IDBCursor had [Symbol.asyncIterator] we could write:

let cursor = await source.openCursor(range).ready;
if (cursor) {
  for await (let c of cursor) {
    console.log(c.key, c.value);
  }
}

... which desugars to:

let cursor = await source.openCursor(range).ready;
if (cursor) {
  let it = cursor[Symbol.asyncIterator](), c, done;
  while (({value: c, done} = await it.next()) && !done) {
    console.log(c.key, c.value);
  }
}

... and you could polyfill this as:

IDBCursor.prototype[Symbol.asyncIterator] = async function* () {
  let c = this;
  do {
    yield c;
    c = await c.continue().ready;
  } while (c);
};

The if (cursor) ... step is a bit awkward. I assume for await (let c of x) throws if x resolves to null?


What if you need to use IDBCursor.continue(key) or IDBCursor.advance(n) ? I think you need to use the desugared iteration and a more complex implementation:

async function() {
  let cursor = await source.openCursor(range).ready;
  if (!cursor) return;

  let it = cursor[Symbol.asyncIterator]();
  let {value: c, done} = await it.next();
  if (done) return;
  console.log(c.key, c.value);

  // continue(k)
  ({value: c, done} = await it.next({key: k}));
  if (done) return;
  console.log(c.key, c.value);

  // advance(n)
  ({value: c, done} = await it.next({advance: n}));
  if (done) return;
  console.log(c.key, c.value);
}

And a more complex implementation polyfill would be:

IDBCursor.prototype[Symbol.asyncIterator] = async function* () {
  let c = this;
  do {
    let op = yield c;
    if ('advance' in op)
      c = await c.advance(op.advance).ready;
    else if ('key' in op)
      c = await c.continue(op.key).ready;
    else
      c = await c.continue().ready;
  } while (c);
};

If you manually call continue() or advance() in the loop things go a bit higglety pigglety.

Also, note that each iterator minted by IDBCursor.prototype[Symbol.asyncIterator] is operating on the same cursor, so it's not a distinct iteration.

... so it probably makes sense to consider making IDBCursor itself an AsyncIterator, where next() is an alias for continue() and advance() exists as a parallel method...

@inexorabletash
Copy link
Owner

inexorabletash commented Jul 22, 2016

So with a different (and incompatible!) set of assumptions:

Handwavy Proposal #2

Assumptions:

Basic use:

let it = await source.openCursor(range).ready;
if (it) {
  for await (let c of it) {
    console.log(c.key, c.value);
  }
}

Advanced use:

async function() {
  let cursor = await source.openCursor(range).ready;
  if (!cursor) return;

  // first value:
  let {value: c, done} = await cursor.next();
  if (done) return;
  console.log(c.key, c.value);

  // second value - next() and continue() should behave identically here
  let {value: c, done} = await cursor.continue();
  if (done) return;
  console.log(c.key, c.value);

  // continue(k):
  ({value: c, done} = await cursor.continue(k);
  if (done) return;
  console.log(c.key, c.value);

  // advance(n)
  ({value: c, done} = await cursor.advance(n);
  if (done) return;
  console.log(c.key, c.value);
}

This can't be polyfilled without additional magic. An approximation would be:

// assume IDBCursor.prototype.[[request]] is an alias for the cursor's associated request.
// assume IDBCursor.prototype.[[used]] is initially false

IDBCursor.prototype[Symbol.asyncIterator] = function() { return this; }
IDBCursor.prototype.next = async function(value) {
  if (!this.[[used]]) {
    this.[[used]] = true;
    return {value: this, done: false};
  }
  return this.continue();
};

let orig_continue = IDBCursor.prototype.continue;
IDBCursor.prototype.continue = async function() {
  orig_continue.apply(this, arguments);
  let c = await this.[[request]].ready;
  assert(!c || c === this);
  return {value: c, done: !!c};
};

let orig_advance = IDBCursor.prototype.advance;
IDBCursor.prototype.advance = async function(n) {
  orig_advance.apply(this, arguments);
  let c = await this.[[request]].ready;
  assert(!c || c === this);
  return {value: c, done: !!c};
};

@littledan
Copy link

littledan commented Jul 22, 2016

All looks very good to me. A couple scattered points:

We could make either option more ergonomic by making the request itself have [Symbol.asyncIterator] on it, which would be responsible for waiting on .ready, checking if that's null, and yielding nothing if it's empty. Why not?

I like the second idea better, all else being equal. .next() returning a Promise makes things more analogous to streams and async iterators, and ultimately, sync iterators. How does that correspond to everyone else's feelings about ergonomics who doesn't have their head buried in the JS specs though?

Maybe we could make .next() return an iteration result and leave advance/continue in place. Also, it might be nice if .next()'s value were not the cursor itself (which "expires" when you advance it again) but a copy of the relevant fields. Of course, this is yet another thing which could make it a little slower.

@inexorabletash
Copy link
Owner

We could make either option more ergonomic by making the request itself have [Symbol.asyncIterator] on it, which would be responsible for waiting on .ready, checking if that's null, and yielding nothing if it's empty.

Seems plausible, iteration would look like:

for await (let r of source.openCursor(query)) {
  console.log(r.key, r.value);
}

We'd have the openCursor() family of methods return an IDBCursorRequest (subclass of IDBRequest, which is compatible).

Polyfill would be:

IDBRequest.prototype[Symbol.asyncIterator] = async function*() {
  let cursor = await this.ready;
  assert(cursor === null || cursor instance IDBCursor);
  if (!cursor) return;
  return cursor.next();
};

I like the second idea better, all else being equal.

Me too! I've added a note to the main proposal pointing at this discussion. Need more feedback from potential users. @jakearchibald ?


Also, it might be nice if .next()'s value were not the cursor itself (which "expires" when you advance it again) but a copy of the relevant fields.

Hrm... plausible. The advantage to keeping the iteration result separate from the iterator is that the values don't mutate when the cursor has advanced. (At least in Chrome we do lazy deserialization at the moment, so the result would probably not be a plain JS object but a host object.)

That would basically be changing this:

  return {value: c, done: !!c};

to:

  return {value: {key: c.key, primaryKey: c.key, value: c.value}, done: false};

...in the polyfill.

Note that the examples of use are already written this way (using the initial cursor result from the request as the iteration control mechanism, and the result c as the holder of the values).

@domenic - can you take a peek?

@jakearchibald
Copy link

I'm happy with async iterables here. I take it advance/delete and break statements all do the right thing here?

@domenic
Copy link

domenic commented Jul 26, 2016

This is all very awesome. I tend to agree with @littledan's points, in particular about abstracting away the .ready promise; that seems like a clear win.

I'm not sure I exactly understand why there's separate .continue() and .next() and .advance() methods; ideally only the standard .next() would be present?

In general I like returning promises instead of IDBRequests in any case; IDBRequests should in my opinion be seen as legacy, even if they've been made thenable.

I also think it would indeed be ideal if the values being iterated over were useful, instead of being the cursor itself. I think if you try to transfer that to the sync analogy, it's pretty clear: it'd be weird to do for-of over an array and get back the array itself.

Is it reasonable to copy ES's values()/keys()/entries() methods here, or is the IDB structure too different from a simple key/value store? If the latter, then the suggested "sequence of { key, primaryKey, value } seems fine. But it sure would be nice for people to be able to transfer their knowledge from other sync and async collection types. And I guess if key and primaryKey are always equal as you seem to suggest, maybe it can just be modeled as keys/values/entries?

@inexorabletash
Copy link
Owner

Thanks for the feedback - keep it coming!

I'm not sure I exactly understand why there's separate .continue() and .next() and .advance() methods; ideally only the standard .next() would be present?

These are existing IDBCursor methods. continue() advances by one record. continue(key) advances to the next record matching key. advance(n) advances n records. One bit from proposal #1 possibly worth keeping: you could pass a dict to next, e.g. next({advance: n}), or next({key: k}).

...if key and primaryKey are always equal...

Only for object store iteration. For indexes they differ.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants