Support ES2015 Sets/iterables in ng-repeat

[qc00]

Currently if I put an ES2015 Set into ng-repeat I get no result (as the instance has no enumerable properties).

I have to use Array.from to convert Sets to arrays all the time. However, this call is not supported by Chrome until recently (even though the Set API existed long ago).

Theoretically this can be a generic implementation for (incl. Arrays), but I forsee issues with change detection, so I guess a Set-specific implementation might be suffice.

[gkalpak]

As explained in #14123 this is not so much about ngRepeat as it is about $watchCollection which is used under the hood.
If we want to support for Set, Map etc, we should probably do it in $watchCollection instead.

[dcherman]

How far do we want to take support for this for? What I mean by that is I was tinkering with supporting Symbol.iterator in both $watchCollection and the ngRepeatAction that handles changes, however IE11 does implement some ES6 data structures (Map, Set), but does not provide any implementation of Symbol, so there's no iteration protocol available.

We could try to fall back to forEach in those cases since it is available, however I'm unsure if we should restrict that operation to only Map and Set, or allow iteration of any object that provides forEach with the assumption that the callback would receive value, key as arguments.

Opinions?

[gkalpak]

We talked about it with @petebacondarwin a couple of weeks ago. We thought about using forEach on every object that has it available, but decided against it. The plan is to (initially at least) support Map and Set only (checking via something like $window.Set && xyz instanceof window.Set) and iterate over values for Set and keys/values for Map.

_{I was supposed to put together a POC PR, but got caught up in other things.
I won't complain if anyone beats me to it 😃}

[mgol]

@gkalpak What's the gain of not supporting all iterable structures?

[gkalpak]

@mgol, one problem is how to detect iterables (e.g. Symbol is not always available/implemented, although Set/Map might be).

Another problem is that different iterables might need to be iterated over differently. E.g. for Set we iterate over the values (similar to how we handle Arrays). The iterator for Map returns a [key, value] pair, but we want to treat it similarly to how we handle Objects, i.e. not use [key, value] as the value, but interpret key as the key and value as the value.

If anyone has suggestions on how to overcome these, that would be awesome, of course 😃

[wesleycho]

Couldn't one declare that if one wants to use iterables such as Set and Map, they use ES compliant versions & if the browser isn't compliant, to use a polyfill as far as any built-in support goes? This would thwart any issues from imperfect implementations found in IE or Safari.

[dcherman]

@wesleycho So the bigger challenge is that as @gkalpak was alluding to, the Symbol.iterator for a given object may not return values that are useful in the context of an ngRepeat for what they want to accomplish.

For example, Array.prototype[Symbol.iterator] returns an iterator containing only the values, not the indexes. Set.prototype[Symbol.iterator] does the same, but we likely want to just provide the value as both the key and value such that the (key, value) notation still works. Map.prototype[Symbol.iterator] on the other hand returns exactly what we need, so that case is easy.

There's also the question of supporting user generated iterables; do we require them to conform to a specific iterator contract for ngRepeat?

We could also choose to not follow the iteration protocol of using Symbol.iterator and instead always use the .entries method if it exists and returns an iterator since that follows what we want in all of the built in cases.

[petebacondarwin]

I think iterating over the result of calling entries (if it exists) sounds like a great idea.

[mgol]

I agree, I like the entries proposal.

[dcherman]

So I started tinkering with this and could use some input on a question related to $watchCollection. Both the new and old values are provided to any registered listeners on change, however the old value is really more of the internal representation so that newValue !== oldValue is always true after the first change.

You can already see behavior where we change an arrayLike item into a true array and pass that as the old value

http://jsfiddle.net/rogqp47y/37/

The question is when it comes to iterables - what do we do? I feel like the expectation can't be that we maintain the behavior of the original collection (Set vs Map vs Array vs others), however there is already precedent with arrayLike items for not providing the same type of collection. We can't simply transform it into an object since a Map with objects as keys is an iterable. As a result, we end up needing to implement something like a Map polyfill in Angular in order to support all the variants of keys that we might run into when providing the old value.

Thoughts?

[gkalpak]

Since there is already the precedent that we convert the passed in value to one of the supported object (i.e. we convert array-like objects to arrays), I think it is reasonable to do the same with iterables.

If we decide to go with entries(), that means that for all (non-array) iterables, we will return the same representation: an array of [key, value] pairs.

Another idea is to have an extra prerequisite, that iterables passed to $watchCollection, should support creating a new instance by calling the constructor with the original iterable as argument. E.g.:

var iter1 = /* some iterable */
var iter2 = new iter1.constructor(iter1);

[petebacondarwin]

Another idea is to have an extra prerequisite, that iterables passed to $watchCollection, should support creating a new instance by calling the constructor with the original iterable as argument.

Is that a common thing? Like would that be following some specification for these kinds of collections?

[gkalpak]

Accepting an iterable as constructor argument is according to the specification for Set, Map, WeekSet and WeakMap, but I don't think it is mandatory for all iterables. (For example this is not the case for Array.)

[graingert]

@gkalpak this would also be good to support Immutable.js objects.

[graingert]

Here's a package that might work for those coming to this issue via google: https://www.npmjs.com/package/ng-next

[bathos]

@gkalpak I like this idea; I think it’s a reasonable constraint / expectation. For example a subclass of Set not maintaining Set’s constructor contract is kind of zalgo-y to begin with, and it’s fair not to support it (since how would you?).

re: WeakMap and WeakSet, I think you probably already know this and were just listing them as examples of the precedent for the new Collection(iter) contract, but just in case, I will point out that they are not iterables.

[tiansh]

Current available "work-around": ng-repeat"item in [].constructor.from(mySet)" // do not use this if you fell it is so ugly