Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add iterator(options) and return [Symbol.asyncIterator] #477

Closed
MeirionHughes opened this issue Sep 8, 2017 · 22 comments
Closed

Add iterator(options) and return [Symbol.asyncIterator] #477

MeirionHughes opened this issue Sep 8, 2017 · 22 comments
Labels
discussion Discussion

Comments

@MeirionHughes
Copy link
Member

MeirionHughes commented Sep 8, 2017

I'd like to propose adding a method to levelup that would allow direct iteration of the underlying store.iterator. This would negate the need for consumers to convert the ReadStream to an Iterable.

from a usage perspective, something along the lines of:

async function main(){
  let db = levelup(encode(leveldown()));

  await db.put("foo", "hello");
  await db.put("bar", "world");

  let search = db.read( { leq: "bar", limit: 10 });

  for await (let data of search){
    let key = data.key;
    let value = data.value;
  }
}

not a huge change to implement this:

LevelUP.prototype.read= function (options) {
  options = extend({ keys: true, values: true }, options)
  if (typeof options.limit !== 'number') { options.limit = -1 }
  return new LevelAsyncIterator(this.db.iterator(options), options)
}

where LevelAsyncIterator can be the stream iterator butchered to remove the Stream aspects and simply implement [Symbol.asyncIterator] instead - along with ability to end (clean) the underlying store.iterator

If people are happy with this proposal I can make the PRs and a new level-iterator repo, based off level-stream-iterator?

I'll do it as a plugin to levelup due to Symbol.asyncIterator being still stage-3.

@MeirionHughes
Copy link
Member Author

MeirionHughes commented Sep 8, 2017

admittedly, you could probably do this without the change:

async function main(){
  let store = encode(leveldown('/path/to/db')))
  let db = levelup(store);

  let search = new LevelIterator(store, { keys: true, values: true, leq: "bar", limit: 10 });

@MeirionHughes MeirionHughes added the discussion Discussion label Sep 8, 2017
@MeirionHughes MeirionHughes changed the title Expose store.iterator(options) as es2015 Symbol.iterator via read method Expose store.iterator(options) as esnext Symbol.asyncIterator via read method Sep 9, 2017
@ralphtheninja
Copy link
Member

Would it be possible/feasible to implement something on top of a ReadableStream? E.g.

async function main(){
  const db = levelup(encode(leveldown()))

  await db.put("foo", "hello")
  await db.put("bar", "world")

  const search = new ReadableStreamIterator(db.createReadStream({ leq: "bar", limit: 10 }))

  for (let data of search) {
    let key = data.key
    let value = data.value
  }
}

This goes against your suggestion but also makes it more generic if it can operate on top of any ReadableStream.

@vweevers
Copy link
Member

vweevers commented Sep 9, 2017

See also: support of Symbol.asyncIterator in Node.js streams (nodejs/readable-stream#254).

@mcollina @calvinmetcalf any thoughts? Until Symbol.asyncIterator lands in Node, do you think levelup should support it on its own? Not through streams, but a lower-level interface. Could be an interesting experiment, also re: performance. Easier to optimize I reckon, without a stream in the middle.

@MeirionHughes
Copy link
Member Author

MeirionHughes commented Sep 9, 2017

You can convert a stream to an asyncIterator, but you potentially have to buffer the stream if it produces faster than the consumer and its in push-mode. I don't like this solution, because the stream is redundent to using the underlying iterator directly and exposing it via Symbol.asyncIterator;

Problem with Symbol.asyncIterator, bar the fact its still stage-3, is that I doubt you'll see it in the wild without polyfill for a good few years; I'm on node 8.4 and even with --harmony the Symbol.asyncIterator global needs to be polyfilled. Even then, without v8 inlining Promises asyncIterator has major performance cost. So I don't recommend adding it to levelup, yet.

In the mean time. I'm just writing a plugin to augment the levelup prototype and give you direct access the iterator, via asyncIterator. Those that want it can just import and use it. Then at a later stage it can be merged into levelup.

I can update the levelup typings to allow merging definitions with third-party plugins to levelup, so the read() method will show up for typescript users.

@vweevers
Copy link
Member

vweevers commented Sep 9, 2017

Problem with Symbol.asyncIterator bar the fact its still stage-3, is that I doubt you'll see it in the wild without polyfill for a good few years.

We can dream :)

SO.. I'm just writing a plugin to augment the levelup prototype and give you direct access the iterator, via asyncIterator. Those that want it can just import and use it. Then at a later stage it can be merged into levelup.

👍

Instead of read(), why not iterator()? In the future, levelup can perhaps expose iterator(options) which simply returns this.db.iterator(options), and then we implement Symbol.asyncIterator in abstract-leveldown. If that makes sense.

And instead of key-value objects, yield arrays, e.g.:

const db = levelup(leveldown('./db'))

for await (let [key, value] of db.iterator()) {
    console.log(key, value);
}

Just spitballing, say we add Iterator.prototype.seek, could still work:

const db = levelup(leveldown('./db'))
const iter = db.iterator()

for await (let [key, value] of iter) {
  if (key == 'something to skip') {
      iter.seek('yonder')
  }
}

@MeirionHughes
Copy link
Member Author

MeirionHughes commented Sep 9, 2017

Yes, ultimately you'd want this in the abstract iterator. But, again, I wouldn't recommend you do this yet.

Sounds good, I'll change what I've got and get some tests going.

@ralphtheninja
Copy link
Member

@vweevers I've never seen the for await construct before. Where can I read more about it?

@MeirionHughes
Copy link
Member Author

MeirionHughes commented Sep 9, 2017

https://github.com/tc39/proposal-async-iteration

this might help if you want to try it: https://stackoverflow.com/questions/43694281/ts2318-cannot-find-global-type-asynciterableiterator-async-generator

@vweevers
Copy link
Member

vweevers commented Sep 9, 2017

It is also available in node 7+ with the --harmony-async-iteration flag.

@vweevers
Copy link
Member

vweevers commented Sep 9, 2017

@ralphtheninja a contrived example:

'use strict'

// Run with: node --harmony-async-iteration example.js

async function example() {
  for await(let [key, value] of new AbstractIterator()) {
    console.log('%s: %s', key, value)
  }

  console.log('done!')
}

class AbstractIterator {
  [Symbol.asyncIterator]() {
    const pairs = [
      ['key1', 'foo'],
      ['key2', 'bar']
    ]

    return {
      next() {
        return new Promise((resolve, reject) => {
          setTimeout(() => {
            const kv = pairs.shift()
            resolve({ value: kv, done: !kv })
          }, 500)
        })
      }
    }
  }
}

example()

@ralphtheninja
Copy link
Member

this might help if you want to try it: https://stackoverflow.com/questions/43694281/ts2318-cannot-find-global-type-asynciterableiterator-async-generator

Upvoted! 😉

@ralphtheninja
Copy link
Member

@vweevers What about the syntax for the AbstractIterator?

class AbstractIterator {
  [Symbol.asyncIterator]() { // <-- this one!
    return {
      next() {
        // ..
      }
    }
  }
}

@vweevers
Copy link
Member

http://2ality.com/2015/02/es6-classes-final.html#computed-method-names. Sans classes you'd write:

AbstractIterator.prototype[Symbol.asyncIterator] = function() {
  return //..
}

@MeirionHughes
Copy link
Member Author

MeirionHughes commented Sep 11, 2017

Proof of concept works nicely and the type merging works to.

image

Bit more refactoring of levelup's typings and a new RC and I'll publish the plugin

edit: I've uploaded the proof of concept: https://github.com/MeirionHughes/levelup-async-iterator and will publish when #481 is pulled

@MeirionHughes MeirionHughes changed the title Expose store.iterator(options) as esnext Symbol.asyncIterator via read method Add iterator(options) and return [Symbol.asyncIterator] Sep 11, 2017
@MeirionHughes
Copy link
Member Author

MeirionHughes commented Sep 14, 2017

@vweevers one hickup with

let db = levelup(encode(leveldown('./db')));

async function main() {
  await db.put("a", "John");
  await db.put("b", "James");
  await db.put("c", "Janet");
  await db.put("d", "Joseph");

  let iter = db.iterator();  

  for await (let [key, value] of iter) {
    if(key === "a"){
      iter.seek("c");
    }
    console.log(key, value);
  }
}

is that skip is not a function on encoding-down's iterator. You'd need a way to pass through functions not present on an iterator instance, from the encapsulated iterator, recursively.

It'll work if you do it manually though: iter.it.seek("c");

output:

D:\Code\levelup-async-iterator>ts-node example
a John
c Janet
d Joseph

@vweevers
Copy link
Member

Oh yes, it was just an illustration; seek is not currently exposed, and only implemented in leveldown. Sorry if I wasn't clear.

@MeirionHughes
Copy link
Member Author

released: https://www.npmjs.com/package/levelup-async-iterator

@vweevers
Copy link
Member

Too much awesome stuff!

@MeirionHughes
Copy link
Member Author

Looks like async iteration is going stable - it is shipped in V8 - so hopefully we'll see it without flags in node 9
tc39/proposal-async-iteration#63 (comment)

@calvinmetcalf
Copy link
Contributor

there is a good chance streams will support async iteration so that's probably a good direction to go in

@huan
Copy link
Contributor

huan commented Sep 22, 2017

+1

@vweevers
Copy link
Member

Breaking this down:

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

No branches or pull requests

5 participants