RubenVerborgh/AsyncIterator

AsyncIterator-ext Package

jeswr opened this issue · 7 comments

jeswr commented

In order to support some more niche use-cases I am coming across I was wondering if it is worth creating an AsyncIterator-ext package (see example below).

IMO the best way to go about this would be:

  • Create an AsyncIterator organisation
  • Move the AsyncIterator repo to a repo in this organisation [Requires @RubenVerborgh due to repo permissions]
  • Create a monorepo of extension components that get released under the @asynciterator/extension-name schema (I am happy to take responsibility for maintenance of this new repo)
  • Additionally export all of the extensions in the package @asynciterator/ext

The downside of having these in a separate package is that it is hard to use these functions as methods of an iterator (as if we just extended the existing AsyncIterator with an AsyncIteratorExt class then using a method like map would return an AsyncIterator class without any of the extension functionality).


An example of an export from that package would be the following maybeIterator function which returns undefined on emptyIterators and an iterator otherwise (the use case for me is to terminate a forward chaining procedure in reasoning components):

import { AsyncIterator } from 'asynciterator';

/**
 * @param source An AsyncIterator
 * @returns The AsyncIterator if it is not empty, otherwise undefined
 */
async function maybeIterator<T>(source: AsyncIterator<T>): Promise<null | AsyncIterator<T>> {
     // Avoid creating a new iterator where possible
  if ((source instanceof ArrayIterator || source instanceof BufferedIterator) && source._buffer.length > 0) {
     return source
  }
  if (source instanceof IntegerIterator) {
     return source.step >= 0 ? source.next > source.last : source.next : source.last
  }

  let item;
  do {
    if ((item = source.read()) !== null)
      return source.append([item]);
    await awaitReadable(source);
  } while (!source.done);
  return null;
}

function awaitReadable<T>(source: AsyncIterator<T>): Promise<void> {
  return new Promise<void>((res, rej) => {
    if (source.readable || source.done)
      res();

    function done() {
      cleanup();
      res();
    }

    function err() {
      cleanup();
      rej();
    }

    function cleanup() {
      source.removeListener('readable', done);
      source.removeListener('end', done);
      source.removeListener('error', err);
    }

    source.on('readable', done);
    source.on('end', done);
    source.on('error', err);
  });
}

I'm not opposed, but I wonder if that's needed.

AsyncIterator should be split into multiple files, and then people can just import what they need?

jeswr commented

As long as you are happy to include that kind of functionality in the main package I'm happy to split the files and add these extensions as separate files once all of the current work around perf. settles down.

If we're talking about something for the next major release, I would also suggest getting rid of commodity prototypal methods entirely as they bind classes to one another, preventing tree-shaking.

commodity prototypal methods entirely

Which ones?

As long as you are happy to include that kind of functionality in the main package I'm happy to split the files and add these extensions as separate files once all of the current work around perf. settles down.

Indeed! Stay tuned; health and time permitting, I'll get to the perf issues and prepare the next release.

Which ones?

@RubenVerborgh the fact that map(), filter(), union() transform(), prepend(), append() and so forth are all defined on the prototype of AsyncIterator means that the respective classes MappingIterator, UnionIterator, TransformIterator, etc... cannot be tree-shaken from one another even when only one of those subclasses is used. AsyncIterator is likely not big enough for this to become an actual issue but, given that we're adding specialized classes for performance reasons, perhaps it'd be a good thing to keep tree-shaking into account.

A potential solution would be to have top-level map(source, fn), filter(source, fn), transform(source, opts) functions, each returning instances their respective subclass. typed-immutable-map, which I've recently forked from an abandoned library and done some work on, uses such an approach.

API-wise this would be a major major breaking change, though. Not sure whether the gains in bundle size would be worth the refactoring costs.

health and time permitting

Hope you're ok! Health comes first!

Thanks! I think it's quite okay and in fact crucial to have these methods, for performance and other reasons. (If they're methods, then subclasses can redefine them for more optional ones.) Some iterators are just core and don't need shaking off (we're talking about 3–4 iterators).