RubenVerborgh/AsyncIterator

[Feature] Overload filter type

jeswr opened this issue · 4 comments

jeswr commented

I think it would be quite useful to have an overload on the filter method for the asyncIterator that is

filter<K extends T>(filter: (item: T) => item is K, self?: any): AsyncIterator<K>;

This would then allow use in cases such as the following:

const termIterator: AsyncIterator<RDF.Term> = /* Some iterator */
const namedNodeIterator: AsyncIterator<RDF.NamedNode> = termIterator.filter<RDF.NamedNode>((term): term is RDF.NamedNode => term.termType === 'NamedNode')

Good idea, yes!

We probably just need to add that single line then? Happy to take a PR.

I just noticed that this breaks a bunch of things in Comunica.
I suspect this is because the overloaded method disappears after compilation.

The compiled .d.ts file only contains the following now:

    filter<K extends T>(filter: (item: T) => item is K, self?: any): AsyncIterator<K>;

While it should contain:

    filter<K extends T>(filter: (item: T) => item is K, self?: any): AsyncIterator<K>;
    filter(filter: (item: T) => boolean, self?: any): AsyncIterator<T>;

Overloads always felt a bit shaky to me, so I've never used them myself, and therefore I have no solution at hand for this.
@jeswr Any idea what may be going wrong here?

@rubensworks I think we explicitly need to add the main function signature too.

jeswr commented

Yeah that's my bad, pretty sure it should be

  filter<K extends T>(filter: (item: T) => item is K, self?: any): AsyncIterator<K>;
  filter(filter: (item: T) => boolean, self?: any): AsyncIterator<T>;
  filter(filter: (item: T) => boolean, self?: any): AsyncIterator<T> {
    return this.transform({ filter: self ? filter.bind(self) : filter });
  }

Might be worth upgrading the tests to TS at some stage to catch this kind of thing.