Skip to content
This repository has been archived by the owner on Oct 8, 2024. It is now read-only.

Consider making the library for Iterables rather than Iterators #8

Closed
Jamesernator opened this issue Jan 19, 2019 · 82 comments
Closed

Comments

@Jamesernator
Copy link

Generally when working with iterables it is nicer to be able to use the result of applying combinators multiple times if the source iterable is re-usable.

For example suppose we have a custom combinator repeat:

function* repeat(iterable, times) {
  for (let i = 0; i < times; i++) {
    yield* iterable
  }
}

// Using the proposed spec's Iterator.map

const seq = Iterator.of(1,2,3)
  .map(x => x**2)

repeat(seq, 2).collect() // [1,4,9] Which isn't really expected

The solution to this is to wrap everything in a [Symbol.iterator]() method:

function repeat(iterable, times) {
  // Where iterable.from creates an object that wraps the
  // Iterable.prototype correctly
  return Iterable.from({
    * [Symbol.iterator]() {
      for (let i = 0; i < times; i++) {
        yield* iterable
      }
    }
  })
}

// Iterable.prototype.map = function map(mapperFn) {
//   return Iterable.from({
//     * [Symbol.iterator]() {
//       for (const item of this) {
//         yield mapperFn(item)
//       }
//     }
//   })
// }

const seq =  Iterable.of(1,2,3)
  .map(x => x**2)

repeat(seq, 2).collect() // [1, 4, 9, 1, 4, 9] as expected

However this is quite tedious, I actually implemented this pattern in my own library and it prevents a lot of footguns especially when implementing more specialised operators.

@devsnek
Copy link
Member

devsnek commented Jan 19, 2019

The iterable protocol is just something that defines how to get an iterator. Iterators themselves are the interface you work with to consume data. Because of this I have to disagree with the approach you take here.

However, I think adding an Iterator.prototoype.repeat would be reasonable, and in fact it is already listed in the prior art section of the readme.

@ljharb
Copy link
Member

ljharb commented Jan 19, 2019

By convention, most iterators are iterable (their Symbol.iterator method does return this), and by convention, builtin APIs tend to operate on iterables and not iterators directly.

@Jamesernator
Copy link
Author

Jamesernator commented Jan 20, 2019

I don't know I'd agree that iterators are what you'd want to work with though. Ultimately it's probably going to be consumed by a for-of (or for-await-of loop), which cannot consume iterators, they consume iterables.

Working with iterators directly can result in footguns too, for example consider an implementation of repeat that only takes an iterator rather than an iterable (which is what the method form is like):

function* repeat(iterator, times) {
  if (times === 0) {
    return
  }

  const stored = []
  while (true) {
    const { value, done } = iterator.next()
    if (done) return
    stored.push(value)
    yield value
  }

  for (let i = 0; i < times - 1; i++) {
    yield* stored
  }
}

We effectively have to store another copy of the values from the iterator, however this isn't good, consider Iterator.range(0, 1000000).repeat(2), this would needlessly create an array with 1000000 items whereas the purely iterable solution of:

Iterable.range = function range(min, max) {
  return {
    __proto__: Iterable.prototype,
    * [Symbol.iterator]() {
      for (let i = min; i < max; i++) {
        yield i
      }
    }
  }
}

const twiceRange = Iterable.range(0, 1000000).repeat(2)

Would not suffer this solution, because once the repetition is done once, it throws the iterator away and creates a new one, in constrast to the purely iterator form which needs to tee anything it might ever want to use more than once.

@devsnek
Copy link
Member

devsnek commented Jan 20, 2019

you don't know that creating the iterator twice will yield the same items every time for repeat. you can't use that technique there. also... you can use iterators with for..of loops.

@Jamesernator
Copy link
Author

No iterators cannot be consumed by for-of loops, Iterator !== iterator:

const iterable = {
  [Symbol.iterator]() {
    let i = 0
    return {
      next() {
        i += 1
        if (i > 3) {
          return { done: true }
        }
        return { done: false, value: i }
      }
    }
  }
}


// Prints 3 numbers
for (const i of iterable) {
  console.log(i)
}

const iterator = iterable[Symbol.iterator]()

// Throws an error
for (const i of iterator) {
  console.log(i)
}

The fact that IteratorPrototype returns this is a convenience method, nothing about the Iterator protocol requires an Iterator to be iterable.


Regarding repeat, my repeat above is closer to itertools.flatten(itertools.repeat(someIterable, 2)) in Python than to itertools.cycle(someIterable).

I'm not sure that there's really a clear intuition to what a name like repeat should do. In my case perhaps a better name would've been flatRepeat.

But even so, it doesn't resolve the range case, if we want a flatRepeat we can't do that without tee-ing in an iterator library.


I don't really see the benefit of an iterator proposal rather than an iterable one. Every single method you could define for an iterator has an equivalent iterable one, for example the common cycle function in itertools libraries could easily be written as:

Iterable.prototype.cycle = function(iterable, times=Infinity) {
  return {
    * [Symbol.iterator]() {
      if (times === 0) {
        return
      }
      let copy = []
      for (const item of iterable) {
        copy.push(item)
        yield item
      }
      for (let i = 0; i < times - 1; i++) {
        yield* copy
      }
    }
  }
}

However we can't do the reverse, there is no similarly efficient version of flatRepeat for an iterator library, we always have to tee the iterator.

This just seems bad having two ways of doing things when people care about iterables not iterators. The only reason the upper majority of people use builtin iterators like array.entries() or the like is because they also happen to be iterable.

@devsnek
Copy link
Member

devsnek commented Jan 20, 2019

@Jamesernator you get an error because you don't inherit from %IteratorPrototype%, which includes [Symbol.iterator]() { return this; } (see the second example in https://github.com/devsnek/proposal-iterator-helpers#example-usage)

You're right that it's not part of the protocol itself, but anyone exposing an iterator manually should do this by extending %IteratorPrototype%. everything in the js builtins does this, everything on the web platform does this, everything in node does this, etc. The reason this proposal exposes Iterator.prototype is because you should do this too.

I don't really see the benefit of an iterator proposal rather than an iterable one

Iterables are just something with Symbol.iterator. All this means is that they can expose a consumable stream of information. Something that is iterable does not have the interface of dealing with the actual stream of information, but rather the api for whatever that iterable does. As an example, arrays, maps, and sets are all iterable, but having a .map for operating as an iterable doesn't match their first class API.

@Jamesernator
Copy link
Author

I will also point out another example of a library that really hasn't been considered in the discussion: RxJS.

While RxJS operators on Observable they have a large number of similarities with iterables. In both structures we have a sequence of values we can consume in some way and would probably want to apply operators over.

In RxJS you do not apply methods to the subscriber (which is analagous to an iterator) even though you could if you wanted to e.g.:

function map(subscriber, mapperFn) {
  return {
    next(value) {
      subscriber.next(mapperFn(value))
    },

    error(err) {
      subscriber.error(err)
    },

    complete() {
      subscriber.complete()
    },
  }
}

const subscriber = {
  next(i) { console.log(i) }
}

someObservable.subscribe(map(subscriber, i => i **2))

Instead you apply the methods to the Observable (analagous to Iterable).

In fact checking it, RxJS even provides a repeat function like I was describing (which is probably where I remember it from as I heavily used Observable in the past). This repeat function consumes the Observable twice, not once replaying the result.

@ljharb
Copy link
Member

ljharb commented Jan 20, 2019

@devsnek right but for..of takes iterables, not iterators, it just happens that builtin iterators all are also iterable.

@devsnek
Copy link
Member

devsnek commented Jan 20, 2019

so are the ones on the web platform, node, and anything created by generators. The only discrepancy is when you write out your own object with next/return/throw.

If the problem here is the definition of the iterator interface, i'm happy also make that more explicit.

@ljharb
Copy link
Member

ljharb commented Jan 20, 2019

Sorry, what i mean is, i agree that it would be foolish to make a non-iterable iterator - I’m saying that it’s maximally useful to make operations that work on iterables and not just iterators.

@devsnek
Copy link
Member

devsnek commented Jan 20, 2019

These also work on iterables... the big constraint here is that the iterable objects have their own first class apis (like array or ReadableStream) so it would make no sense to coerce them into also supporting an entire set of methods to operate on iterators.

@ljharb
Copy link
Member

ljharb commented Jan 20, 2019

By “work on iterables” it would probably be sufficient if the first thing it did is GetIterator on the iterable (which might already be an iterator), and then operated only on the resulting iterator.

@rbuckton
Copy link

I'm generally opposed to extending %IteratorPrototype% for this, I feel it is the wrong extensibility point as iterators are consuming, i.e. once you run an iterator to completion it cannot be re-iterated. Prior art like .NET's System.Linq.Enumerable operate at the IEnumerable level (equivalent to an ECMAScript "iterable"), as do libraries like lodash and itertools (and my own iterable-query package). Unfortunately, %GeneratorPrototype% suffers from the same issues as %IteratorPrototype%.

I have written a fair amount of code that depends on querying "iterables" that would not work for "iterators". As a result, I most likely wouldn't be able to use this feature if it were to make it into the language and I would still need to depend on 3rd party libraries.

In general I would prefer to see these as functions that could be used in conjunction with either the pipeline operator (|>) or bind operator (::) proposals, or barring that an Iterable base-class or "mix-in" that could be used with built-ins and userland code, even if that means waiting on this proposal (or a variant of this proposal) until one of those options becomes standardized.

@devsnek
Copy link
Member

devsnek commented Jan 31, 2019

@rbuckton can you just outline quickly what the semantics look like? my understanding is that for every iterator you then need to create this "iterable" wrapper around it which you then chain methods off of. is this correct?

the reason i'm hesitant about this is because of the interaction with iterators that i've seen in the ecosystem. Things tend to expose one-shot iterators, rather than iterables (generators, map.prototype.keys, web and node apis, etc, all work like this)

I'm very open to changing this, but personally i'm still unconvinced that the change makes sense for javascript, regardless of what other languages do.

@rbuckton
Copy link

@devsnek: For this proposal to work, you have to create an "iterator" wrapper around it as well, so it is not much different.

For example, consider filter and filterBy in iterable-query:

For the pipeline proposal (F# with partial application), you would use filter like this:

import { fn } from "iterable-query";
const odds = [1, 2, 3] |> fn.filter(?, x => x % 1 === 1);
for (const x of odds) console.write(x);
const sum = odds |> fn.sum();
console.log(sum);

If not using pipeline, you can do the same thing with a Query:

import { from } from "iterable-query";
const odds = from([1, 2, 3]).filter(x => x % 1 === 1);
for (const x of odds) console.write(x);
const sum = odds.sum();
console.log(sum);

// alternatively:
// import { Query } from "iterable-query";
// const odds = Query.from([1, 2, 3]).filter(x => x % 1 === 1);
// ...

If we were using iterator, sum would be 0. While this is a somewhat contrived case, I have real-world code that depends on being able to restart iteration.

If map.keys()[Symbol.iterator]() had been specified to return a fresh iterator rather than this, I might be more supportive of this proposal. Unfortunately, that would be a breaking change. Perhaps we could add something like a @@newIterator method that these methods could look at first if it existed, and fall back to @@iterator if it did not? @@newIterator would be expected to always return a fresh iterator, and to either not exist or perhaps return null if such freshness is not possible? That might remove some of my concerns.

@domenic
Copy link
Member

domenic commented Jan 31, 2019

Upon reflection, this issue is basically what I was getting at with #5, and with the introduction to the TC39 presentation which Ron attended. JavaScript has iterators; they are here to stay, and are useful, and many of us are able to use them without encountering all the issues that some folks here describe. This proposal is about making them more useful.

Separately, folks should also work on a proposal to make the language's various iterables (so far Map, Set, Array) more useful. Those are good things to do: as mentioned, it's much nicer to be able to do const newMap = myMap.filter(entry => ...) than to have to do const newMap = new Map(myMap.entries().filter(entry => ...)).

I definitely encourage people to work on such a proposal for iterables. However, this proposal is focused on making iterators more useful. I think it would be a huge shame if folks blocked work on improving iterators, because in their code they prefer not to operate on them. I would rather have complementary efforts that do not block each other.

@devsnek
Copy link
Member

devsnek commented Jan 31, 2019

@rbuckton if you create your iterable from a given stateless resource such as an array that works fine, but the most common case is that all you have is an iterator, such as from invoking a generator or calling Map.prototype.keys.

function* someNumbers() {
  let i = 0;
  while (i < 100) yield i++;
}

const odds = Iterable.from(someNumbers()).filter((x) => x % 1 === 1);
for (const x of odds) console.log(x); // 1, 3, 5, ...
console.log(odds.sum()); // still 0?

If map.keys()[Symbol.iterator]() had been specified to return a fresh iterator rather than this, I might be more supportive of this proposal.

I feel exactly the opposite. If it returned a fresh one I would want your API, but because it doesn't i think the prototype api makes more sense.

@rbuckton
Copy link

Out of curiosity, how web-incompatible would it be to have Array, Map, and Set inherit from an Iterable base class/%IterablePrototype% prototype?

@domenic
Copy link
Member

domenic commented Jan 31, 2019

I would be very surprised (but pleasantly so) if making changes to Array was doable at all.

@rbuckton
Copy link

If we use %IteratorPrototype% for this, my only alternative will be to do this:

const odds = () => [1, 2, 3].values().filter(x => x % 1 === 1);
for (const odd of odds()) console.log(odd);
const sum = odds().sum();
console.log(sum);

It's not too terrible, but I'm still not a big fan.

For a generator function, you are correct that it would always be consuming regardless. Most of the times I've implemented a generator as part of an API, however, its been something like this:

class C {
 *[Symbol.iterator]() {
  yield 1;
  yield 2;
 }
}
const obj = new C();
for (const x of obj) ...
for (const x of obj) ... // restarts iteration from the beginning.

The fact that the majority of the prior art linked in the explainer point to APIs that work at the "iterable" level seems to be a strong indicator that "iterator" is the wrong place.

@devsnek
Copy link
Member

devsnek commented Jan 31, 2019

@rbuckton the prior art exists to suggest that there is a precedent of working with language constructs that represent an ordered pull stream of values.

in javascript specificlly, as you have helpfully demonstrated, this happens to usually be the iterator. people don't write class c { *[Symbol.iterator]() { ... } } they write function* c() { ... }. Whether that was a good or bad design choice is definitely out of scope of this proposal.

@rbuckton
Copy link

The fact that you can create a generator that produces a consuming iterator isn't the issue (i.e. the design choice was fine). C# works effectively the same way:

class C : IEnumerable { // new C() is Enumerable
  public IEnumerator GetEnumerator() { // analogous to [Symbol.iterator]
    yield return 1;
    yield return 2;
  }
  …

  IEnumerable CustomEnumerator() { // analogous to function*
    yield return 1;
    yield return 2;
  }
}

Right now the C# and ES implementations (with respect to iteration only) are roughly the same. The place where they would diverge is if we choose to add these methods to %IteratorPrototype% rather than some other construct.

@devsnek
Copy link
Member

devsnek commented Jan 31, 2019

@rbuckton Right.... I'm adding things to the enumerator here, not the enumerable. That's the same in C#. All the helpers in C# are on IEnumerator.

@rbuckton
Copy link

rbuckton commented Jan 31, 2019

@devsnek
Copy link
Member

devsnek commented Jan 31, 2019

@rbuckton it seems we were looking at different interfaces. I'm looking at the more generic System.Collections.IEnumerable and System.Collections.IEnumerator.

In the case of System.Linq.*, I disagree that the design of those matches the design of what js has, so i think its kind of irrelevant to this case. If JS was designed like System.Linq, setInstance.map would be an iteration helper, which just isn't how things work in this ecosystem.

Again, I don't think the overarching design of iteration in JS is within scope here.

@Jamesernator
Copy link
Author

Jamesernator commented Jan 31, 2019

I will point out something else that could be done that would allow the same methods to be used on both iterators and iterables would be something like the protocols proposal.

If we have an abstract method that needs to be implemented (e.g. [Iterable.toType] then we could just call that with the result of applying a given method to an iterator. All Iterable's could then just implement that protocol.

As a concrete example supposing Array/Iterator/Set/etc were classes:

class Array implements Iterable {
  [Iterable.toType](iterableIterator) {
    return Array.from(iterableIterator)
  }
}

class Set implements Iterable {
  [Iterable.toType](iterableIterator){
    return new Set(iterableIterator)
  }
}

class Iterator implements Iterable {
  [Iterable.toType](iterable) {
    return iterable[Symbol.iterableIterator]()
  }
}

The implementation of methods would look something like this:

function* _mapImplementation(iterable, mapperFn) {
  for (const item of iterable) {
    yield mapperFn(item)
  }
}

function* _repeatFlatImplementation(iterable, times) {
  for (let i = 0; i < times; i++) {
    yield* iterable
  }
}

const Iterable = makeProtocolSomehow({
  requiredMethods: {
    toType: Symbol('Iterable.toType'),
  },
  methods: {
    map(mapperFn) {
      return this[Iterable.toType](_mapImplementation(this, mapperFn))
    },

    repeat(this) {
      return this[Iterable.toType](_repeatFlatImplementation(this, times))
    },
  },
}

Not only would the methods just work on Iterator but they also would work on all current and future iterables.

@rbuckton
Copy link

We are not looking at different interfaces, per se.

  • System.Collections.IEnumerator is the weak (non-generic) form of System.Collections.Generic.IEnumerator<T> and is analogous to the Iterator interface defined in the ECMAScript spec.
  • System.Collections.IEnumerable is the weak (non-generic) form of System.Collections.Generic.IEnumerable<T> and is analogous to an "Iterable" (i.e. an object that defines an @@iterator method) in the ECMAScript spec.
  • System.Linq.Enumerable exposes "extension methods" that perform various iteration helpers over an IEnumerable<T>, which provides the same capabilities to .NET/C# that this proposal seeks to add to ECMAScript.

An IEnumerable in .NET is not required to return a fresh IEnumerator, but most built-in collections do. System.Linq.Enumerable interacts with IEnumerable and supports iterating over "fresh" IEnumerator instances of the source if they are provided, as that is inherent in the call to GetEnumerator (nee @@iterator in ECMAScript).

If System.Linq.* is irrelevant, it shouldn't be listed as Prior Art. However, I strongly believe that it is relevant to this discussion and the proposal in general.

@rbuckton
Copy link

@Jamesernator: That goes to my point that a "mixin" (or protocol) approach seems like a better alternative to me than putting these on %IteratorPrototype%.

@devsnek
Copy link
Member

devsnek commented Jan 31, 2019

@rbuckton again, prior art is listed to show the usage of working with ordered pull streams of data in other languages. For example, i linked rust, but this proposal is not suggesting that iter::Peekable should be added (although it would be cool if it was, idea for another proposal there 😄).

I'm really confused about how you're viewing the usage of existing apis within the ecosystem, could you provide some examples of things like creating iterators and using iterators and etc under your view?

@tabatkins
Copy link

Agreed overall with @bakkot and @domenic:

  1. Iterators do iteration; iterables produce an iterator. Thus, methods that rely on iterating should be on the iterator, not the iterable, at least by default.
  2. Specific iterables can implement these on themselves fairly trivially with just filter(...args) { return Foo.from(Iterator.from(this).filter(...args)); }, etc.
  3. The protocols proposal, when it stabilizes/advances, is the right way to add generic behavior to a bunch of disparate classes; prototype hacking is fragile and will usually run into webcompat problems for existing classes. This is clearly a "multiple inheritance" issue, which prototype chains are unsuited to handling cleanly. At that point, you can opt your class into the iterator methods with a trivial class Foo implements Iterator { addition to your class head, and presumably receive the free default implementation outlined in the previous bullet point. (Probably just depending on the class implementing [Symbol.iterator] and a new [Symbol.fromIterator].)
  4. Specific iterables can also often implement more efficient behaviors for the methods than the generic iterator interface allows. I expect myMap.filter() to have a more efficient impl than the default would give, since it wouldn't be casting back and forth between types.

@rauschma
Copy link
Contributor

rauschma commented Feb 16, 2019

I find it important to keep iterables and iterators separate, conceptually. IME, mixing different concepts eventually leads to problems.

Arguments against iterator methods:

  • All constructs such as Array.from() and for-of work with iterables (not iterators).
    • Similarly, in all languages that I’m aware of: If they have utility operations for iteration then those work with iterables (domain and range).
  • Putting functionality into an abstract iterator “class” means that you can’t do lightweight iterators, anymore. You always have to extend that class.
  • Always having to create iterators, is inconvenient:
    // Iterator method:
    const result1 = [...arr1.values().zip(arr2.values(), arr3.values())];
    
    // Iterable method:
    const result2 = [...arr1.zip(arr2, arr3)];
    
    // Function:
    const result3 = [...zip(arr1, arr2, arr3)];

Additional benefits of functions:

  • Libraries will provide additional operations for working with iterables. If all iterable-related functionality are functions, code looks more consistent.
  • For n-ary operations (such as zip()), I prefer functions over methods, because there is no clear dominant operand that could become the receiver.

There will probably also be operations for creating iterables. Those will also be functions (or static methods). For example: range(start, end)

@devsnek
Copy link
Member

devsnek commented Feb 16, 2019

if were we designing iteration in JavaScript from scratch this might have some bearing but the way that iteration exists /now/ in JavaScript doesn't work like that. we cannot put the methods directly on the things that expose iterators because 1) it's not a consistent interface (generators, the absolute primitive of creating iteration interfaces, would be left out) and 2) that space is already taken. Array.p.map/forEach/etc exists, Map.p.forEach,etc and I want these additions to be actually usable.

separate functions is another idea that was discussed in another issue, and we decided not to because the current JavaScript way is by using inheritance, and we have no good method of delivering random functions without depending on other language features that are not even certain to exist.

I understand a lot of people dislike the design of iteration in JavaScript, but this is not a proposal to fix your issues with iteration, it's a proposal to add helper methods to the consistent interface that JavaScript uses for iteration.

@rauschma
Copy link
Contributor

separate functions is another idea that was discussed in another issue, and we decided not to because the current JavaScript way is by using inheritance, and we have no good method of delivering random functions without depending on other language features that are not even certain to exist.

How about namespace objects? Key precedent: Math (whose static methods were not added to Number.prototype)

I understand a lot of people dislike the design of iteration in JavaScript, but this is not a proposal to fix your issues with iteration, it's a proposal to add helper methods to the consistent interface that JavaScript uses for iteration.

In my understanding, we are discussing how to best implement helper operations for iteration.

@devsnek
Copy link
Member

devsnek commented Feb 16, 2019

I'm also concerned about having to use pipelines because they aren't near finalization, and there's no precedence for anything in the js standard library using fp. if pipelines magically land in the next year and the committee decides to ship stdlib functionality as fp, I'd be happy to revisit the topic.

for now though, f().map().forEach is the way everything else in js stdlib works, so it's the pattern I would like to stick to.

in my understanding, we are discussing how to best implement helper operations for iteration.

it's pretty much just been someone trying to convince that c# has a better pattern for iteration and we should use this as an opportunity to change what idiomatic iteration is in js to match c#. if that wasn't the train you were jumping on I'm sorry for assuming so. In either case, the issues with an iterable interface have been outlined a quite a lot in the previous discussion.

@devsnek
Copy link
Member

devsnek commented Feb 16, 2019

I've noted the constraints here: https://github.com/tc39/proposal-iterator-helpers/blob/master/DETAILS.md#interface-constraints. Anyone should feel free to suggest additional constraints for this list.

@rauschma
Copy link
Contributor

rauschma commented Feb 16, 2019

it's pretty much just been someone trying to convince that c# has a better pattern for iteration and we should use this as an opportunity to change what idiomatic iteration is in js to match c#. if that wasn't the train you were jumping on I'm sorry for assuming so. In either case, the issues with an iterable interface have been outlined a quite a lot in the previous discussion.

Not my train at all! I’m mostly happy with the current protocol.

I'm also concerned about having to use pipelines because they aren't near finalization, and there's no precedence for anything in the js standard library using fp. if pipelines magically land in the next year and the committee decides to ship stdlib functionality as fp, I'd be happy to revisit the topic.

  • Even without the pipeline operator, I prefer functions. YMMV, but having to create iterators first and having to extend an Iterator class when implementing an iterator, would affect my code more negatively than not being able to chain.
  • Precedent for using a function to map: Array.from(). JavaScript has always been a mix of OOP and FP.

for now though, f().map().forEach is the way everything else in js stdlib works, so it's the pattern I would like to stick to.

With the proposal,

  • it wouldn’t be iterable.map().forEach();
  • it would be iterable.values().map().forEach().
  • or iterable[Symbol.iterator]().map().forEach()

@devsnek
Copy link
Member

devsnek commented Feb 16, 2019

With the proposal, ...

Yes. What you list is how the stdlib, userland, browsers, node, etc, work as that that is the design that was created when iteration was added to the language.

This issue thread seems very focused on "given an object i know nothing about, how can i iterate from it" but that isn't anywhere near the common case. It is an absolutely valid case, and I have added fromSync and fromAsync (still named from in the readme and polyfill) to handle this case gracefully, as you're right that just directly calling Symbol.iterator isn't ideal.

The common case in libraries, the stdlib, node, the web, etc, is functions that expose iterator interfaces. This, for better or worse, is the idiomatic way to expose your iteration to the world. (array.values, map.entries, headers.entries, etc)

having to create iterators first and having to extend an Iterator class when implementing an iterator, would affect my code more negatively than not being able to chain.

If you want to manually create iterators you're already in for a bad time. This proposal, in fact, only makes it easier. The current way to inherit from %IteratorPrototype% is very annoying (and note that if you aren't doing that today, your iterator will be second class in the ecosystem, because every other iterator coming from node, the web, stdlib, etc, already inherits from %IteratorPrototype%). Personally I prefer return Iterator.fromSync({ next() {} }) over return Object.setPrototypeOf({ next() {} }, Object.getPrototypeOf(Object.getPrototypeOf([][Symbol.iterator]()))) but that's just me.

If you make iterators with generators, which is what most people do, you will get a bunch of helpful methods for free.

@domenic
Copy link
Member

domenic commented Feb 16, 2019

I agree with @devsnek. I think there are room for multiple proposals in this whole space, and I do think the readme should be explicit (I originally opened #5 in a similar vein). I could see non-overlapping proposals for:

  • Making iterators more useful (this proposal), thus benefiting the values()/keys()/entries() of every collection, as well as generator/async generator instances, and other single-consumption cases like web streams.
  • Making collections (iterables) more useful, by adding same-named methods to all of them, or using protocols or similar, so that you can directly operate on Maps/Sets/etc. and get back a result of the same type, when that's what you want to do. I think this is of competitive usefulness to the current proposal, but it's technically much more challenging with more open questions and mechanisms.
  • FP operations, probably on iterables, provided as separate standalone functions instead of methods.

I think the FP operations approach is novel to the JS standard library, which has been object oriented so far, and so faces an uphill battle. I think it's intimately tied to the question of whether the pipeline operator (in its many variants) will make progress in committee.

But most importantly, it's completely separable from this proposal; I see no reason why we couldn't have both.

I think we should document these complementary approaches in the readme, and let this proposal focus on its specific strengths and use cases, of improving the iterator/async iterator ecosystem.

@rauschma
Copy link
Contributor

rauschma commented Feb 16, 2019

@domenic: OK. A separate proposal makes sense.

@devsnek:

The common case in libraries, the stdlib, node, the web, etc, is functions that expose iterator interfaces. This, for better or worse, is the idiomatic way to expose your iteration to the world. (array.values, map.entries, headers.entries, etc)

Note that these functions return iterables – either Arrays (Object.keys()) or iterators that are also iterables (.entries()). Otherwise, they wouldn’t work with for-of and spreading.

If you want to manually create iterators you're already in for a bad time.

I’ve done this a few times (without inheriting from %IteratorPrototype%) and have never had any issues. The proposal would break some of my code (in that it wouldn’t work as expected, anymore).

@devsnek
Copy link
Member

devsnek commented Feb 16, 2019

i'd argue it already doesn't work as expected, but that's because the design of iterators in js is a bit wonky, not your own fault. Like i said above, one of the efforts here is to make it easier to return iterators that adhere to all the things that make them usable in the ecosystem (like also being iterable)

@pemrouz
Copy link

pemrouz commented Jul 23, 2019

I think different proposals here conceptually makes sense as @domenic mentioned (helpers for iterators, iterables and standalone functions).

However, that doesn't mean we should do all of them, at least not spray all the prototypes all at the same time. My thinking is that we should start with the more general abstraction, and then we can rationalise and add specific prototype methods in terms of the more general approach as convenient/ergonomic shortcuts based on empirical usage.

There's also a lot of data points and good feedback here which is being unreasonably dismissed or misrepresented:

  • It's a major wart to require users to change the prototype of objects as a way to be able to use these library helpers i.e. Object.setPrototypeOf(MyIteratorPrototype, Iterator.syncPrototype). We don't have this pattern in any other part of the language.

  • If it's a subclass, it's even impossible.

  • It's awkward to expose, not one, but two new different globals just for this purpose.

  • It's a new footgun to complicate iterators such that they could inconsistently not inherit from the right prototype.

  • The prototype approach does not provide a way for users to extend and write their own operators. The logical extension of users wanting to extend by extending the prototype would be very bad.

  • When the costs and shortcomings of the prototype approach are given, it's ludicrous to misrepresent the standalone functions as a somehow foreign FP thing vs "traditional JS". In the history of JS, there are plenty of non-prototype examples, Object.assign, Promise.all, Math, as many keep pointing too.

  • The community is unanimous in using standalone functions. If there ever was a cowpath to pave, this is it. None of the libraries listed are using the prototype approach. Not even early pioneers like lodash with Array's (i.e. _.map), not even the main one listed here (itertools).

  • Other languages like Python also don't take this approach.

  • It's not accurate to say standalone functions depends on the pipeline proposal. Again, this is what the community has been and is already doing today without it. As @rauschma showed, it's even more ergonomic in real world code. The fact that it works with function composition, is an added bonus, and also more future-proof.

  • When people point out the inconsistencies between iterator/iterable, it seems silly to try to paint them as wanting to "change how iteration works". In fact, you are literally changing how this works: Consider splitting this as two proposals: one proposal to enforce all iterators in JavaScript to inherit from Iterator.syncPrototype and Iterator.asyncPrototype. Then a follow up proposal to add your helper functions to those prototype. The first is clearly changing how the otherwise now lightweight iteration protocol works in JavaScript.

  • More importantly, usage of this iterators-as-operators approach is effectively zero across the board. This is because it's a premature abstraction, and if you use it across a few more use cases it's very easy to see how it will evolve.

Hence my suggestion here is to do the following:

  • Standardise standalone functions that work on iterables
  • Wait to see if there is still a demand: Standardise the non-lazy collection methods version that apply the same transforms and then serialise a new collection if there is a need as outlined in tc39/proposal-emitter#16
  • Wait to see if there is still a demand: Standardise helpers on iterators (as two proposals, change iterators to inherit, then add the same helpers)

However, there is practically zero demand/usage for this already, and once we pave the way for developers to work with iterables, it will be even less likely that we need this.

@devsnek
Copy link
Member

devsnek commented Jul 23, 2019

just to clarify, setting the prototype is pretty much already a requirement. ecma262 (which means all user written generators), the web, nodejs, etc, all do this, regardless of whether you think it was a good design choice. obviously you can make an iterator that doesn't have the prototype, but you won't be able to pass it to for-of loops and such and i think people using your code would be annoyed in that case.

@pemrouz
Copy link

pemrouz commented Jul 23, 2019

obviously you can make an iterator that doesn't have the prototype, but you won't be able to pass it to for-of loops and such and i think people using your code would be annoyed in that case.

image

all do this, regardless of whether you think it was a good design choice

Please stop trying portray anyone who disagrees with this proposal as disagreeing with how iteration works in JavaScript, and hence, you must be right 🙄. This isn't remotely true.

In fact, as aforementioned, you are the ones currently trying to change it, and the non-prototype based iterators are working great as-is.

@devsnek
Copy link
Member

devsnek commented Jul 23, 2019

you passed an iterable, i'm talking about iterators. you have to either have a [Symbol.iterator]() { return this; } or inherit from %IteratorPrototype%, which has [Symbol.iterator]() { return this; }.

i just wanted to clarify that point, i don't think i'm always right, and i do agree with many of the other points you made.

@pemrouz
Copy link

pemrouz commented Jul 23, 2019 via email

@ljharb
Copy link
Member

ljharb commented Jul 23, 2019

Yes, but by convention, all iterators are themselves iterable, and inherit from IteratorPrototype - just because you can make something that ducktypes successfully doesn't mean that's the proper or best or ideal way to make a thing.

You can make a thenable that's not a Promise, but everything in the language eagerly converts it into a proper Promise as soon as it touches it.

@Jamesernator
Copy link
Author

You can make a thenable that's not a Promise, but everything in the language eagerly converts it into a proper Promise as soon as it touches it.

The difference is that precisely no functions in ecma262, the web platform or Node actually accepts an "Iterator" as defined by the iterator interface, everything that uses iterators only accepts "Iterables" (also as defined by the interface) and pulls out an iterator from it.

Some of these "Iterables" happen to be things that inherit "IteratorPrototype" and so get a [Symbol.iterator]() method which is fine for convenience but doesn't mean those methods accepted "Iterators" (which again has a canonical definition that does not require self-iterable-ness).

@pemrouz
Copy link

pemrouz commented Jul 23, 2019 via email

@domenic
Copy link
Member

domenic commented Jul 23, 2019

It's a major wart to require users to change the prototype of objects as a way to be able to use these library helpers i.e. Object.setPrototypeOf(MyIteratorPrototype, Iterator.syncPrototype). We don't have this pattern in any other part of the language.

Creating custom iterators is a power tool. Almost all iterators will be created by using generator functions, or chaining off of the existing ones (returned by Map/Set/etc.)

It's a new footgun to complicate iterators such that they could inconsistently not inherit from the right prototype.

This proposal makes that strictly better, by making it easier for iterators to inherit from the right prototype.

The prototype approach does not provide a way for users to extend and write their own operators. The logical extension of users wanting to extend by extending the prototype would be very bad.

This is the same situation as Array today.

When the costs and shortcomings of the prototype approach are given, it's ludicrous to misrepresent the standalone functions as a somehow foreign FP thing vs "traditional JS". In the history of JS, there are plenty of non-prototype examples, Object.assign, Promise.all, Math, as many keep pointing too.

This is not an accurate read of those examples.

  • Object.assign: extending Object.prototype is impossible for compatibility reasons, so we have to use static.
  • Promise.all: combinators of many instances as static methods is the pattern here.
  • Math: what instance would this even operate on.

In contrast, the precedent of Array/Map/Set is strong. When you create something derived from an existing item, you use instance methods.

Also, please watch your tone; "ludicrous" and "misrepresent" are very strong words to be throwing around.

The community is unanimous in using standalone functions.

This is not true.

The community has wisely decided not to extend built-in prototypes. We, as language designers, have that ability, and they've been looking to us for guidance. Indeed, when we've added new proposals to Array, e.g. find/findIndex (ES2016), incldues (ES2016), they've been instance methods.

Furthermore, the community has used the prototype pattern in popular libraries like lodash. They have a clever hack to avoid modifying the prototype: wrapper objects. You do _(array).pick(...), instead of array.pick(...), but it still uses the familiar, prototypal, left-to-right composition.

one proposal to enforce all iterators in JavaScript to inherit from Iterator.syncPrototype and Iterator.asyncPrototype.

They already do. You can break the contract that the language enforces for all its built-ins if you really want to, just like how you can create custom objects that don't inherit from Object.prototype. But all objects created using object literal syntax inherit from Object.prototype, as do all the built-in objects. Similarly, all iterators created using "(async) iterator literaly syntax" (i.e., (async) generators) inherit from Iterator.prototype, as do all the built-in iterators.

Then a follow up proposal to add your helper functions to those prototype.

That is this proposal.

More importantly, usage of this iterators-as-operators approach is effectively zero across the board.

As noted above, this misrepresents the situation; popular libraries like lodash already inhabit this space, but wisely use wrappers instead of extending built-in prototypes.

Standardise standalone functions that work on iterables

I am opposed to any proposal that uses standalone functions instead of the prototypal method tradition used by Array, Map, Set, etc.

Wait to see if there is still a demand: Standardise the non-lazy collection methods version that apply the same transforms

These generally all already exist. Array.{map, filter, reduce, forEach, some, every, find} are all present. The remaining parts of this proposal, namely take, drop, asIndexedPairs, and collect, only make sense on lazy collections. (Well, asIndexedPairs perhaps we could make your argument for. I would be OK dropping that from this proposal and pursuing it on Array first.)

Wait to see if there is still a demand: Standardise helpers on iterators (as two proposals, change iterators to inherit, then add the same helpers)

As noted previously, you seem to misunderstand how the language already inherits from iterators. And, I think the existence of libraries like lodash, or just the tons of code that has to convert to an Array (via [...set.values()].map(...)), is sufficient evidence of demand.

JavaScript has an iteration protocol. Not an Iterator class. The point was there is no need to inherit from any prototype right now. This proposal changes those semantics.

This proposal makes the Iterator prototype analogous to the Array prototype, in that it gets useful methods. Even though the "has a length property" exists, we still add things to the Array prototype, because of all the nice, built-in, language-supported ways of creating arrays that inherit from %ArrayPrototype%, and how we want to make arrays nicer to use. Similarly, given all the nice, built-in, language supported ways of creating iterators that inherit from %IteratorPrototype%, we should add methods to them, to make them easier to use.

@tabatkins
Copy link

Creating custom iterators is a power tool. Almost all iterators will be created by using generator functions, or chaining off of the existing ones (returned by Map/Set/etc.)

I disagree with this pretty strongly. My own experience with the iterator protocol in PHP is that I used it pretty heavily in my own classes; pretty much any collection wants to implement it.

Can you rely on generators? Sure; you can always have a .items()/etc generator method. But it's very nice to be able to pass a collection directly to a loop, as all the built-ins do.

Functions that return iterators will basically always be generators, sure. But collections are a totally different story.

@devsnek
Copy link
Member

devsnek commented Jul 23, 2019

collections aren't iterators, they're iterables, and *[Symbol.iterator]() {} is both possible and a popular choice.

@tabatkins
Copy link

Sigh, right. The difference keeps catching me.

@devsnek
Copy link
Member

devsnek commented Jul 25, 2019

We have chosen to move forward with the iterator approach, so I'm going to close this out.

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

9 participants