Allow {{each-in}} to iterate ES6 Maps on browsers with support

[cibernox]

@rwjblue It's not too hard, but I was thinking that we should probably just allow to iterate anything that was @@iterator support, so keep this as a WIP.

I mostly want feedback on the tests, the implementation might change.

[rwjblue]

tests seem reasonable, but I am not so sure about the implementation side

[cibernox]

There is two possibilities to implement this.

The first one is to detect Map support, check if the iterable is a Map, and use Map.prototype.forEach if it is, and then have another path for objects.

The second one is to detect if the element is an iterable (has the @@iterator symbol), but this leaves out Internet Explorer 11, that has support for forEach on maps but knows nothing about symbols.

I went with the first one, as IE11 still matters. If we want to support iterables, we might need three branches, one for Maps and Sets that supports IE11, another for Objects and Arrays and a third one that will only work on browsers with Symbol support that checks for @@iterarator.

Sounds reasonable?

[rwjblue]

I left an inline suggestion for an alternative way to handle this (without requiring instanceof checks for specific object types).

[rwjblue]

Not sure you need window. here?

[rwjblue]

My thought was that we could simplify this a bit. Basically, use native symbol when possible then fallback to .forEach if present, else do Object.keys. And avoid directly doing instanceof checks here.

const HAS_NATIVE_SYMBOL = (function() {
  let hasSymbol = typeof Symbol === 'function';
  if (hasSymbol === false) { return false; }

  let instance = Symbol('test-symbol');
  // use `Object`'s `.toString` directly to prevent us from detecting
  // polyfills as native
  return Object.prototype.toString.call(instance) === '[object Symbol]';
})();

// ...snip...

if (iterable && (typeofIterable === 'object' || typeofIterable === 'function')) {
  let keys;
  let values;

  if (HAS_NATIVE_SYMBOL && typeof iterable[Symbol.iterator]) {
    keys = [];
    values = [];
    let iterator = iterable[Symbol.iterator]();
    
    while(true) {
      let result = iterator.next();
      if (result.done) break;
      let [key, value] = result.value;
      keys.push(key);
      values.push(value);
    }
  } else if (typeof iterable.forEach === 'function') {
    keys = [];
    values = [];
    iterable.forEach((value, key) => {
      keys.push(key);
      values.push(value);
    });
  } else {
    keys = Object.keys(iterable);
    values = keys.map(key => iterable[key]);  
  }
}
// ...snip...

[rwjblue]

Also, its likely possible to create a custom iterator object that avoids eagerly iterating all the items (like I did in that snippet), but we would need to pester some folks to get some clarity on the "right" way to do it.

[cibernox]

@rwjblue I've applied your feedback with very minor adjustments.

It turns out that adding support for iterables added a features I wasn't counting with.
It turns out that if every time the next() function of an iterator is called it returns a an array, the first element of that array is considered the key and the second the value.

That means that {{each-in}} can iterate an iterable like this:

    let iterable = {
      [Symbol.iterator]: () => makeIterator([['one', 'foo'], ['two', 'bar']])
    };

and it would be like iterating the object { one: 'foo', two: 'bar' }.

I added tests for that feature, but if that is not a behaviour we want to support I can remove the tests.

[cibernox]

After this I could add support for iterables in {{#each}}, either in this same PR or in another one. Obviously with iterators {{each}} wont be able to perform KVO, but as long as it's documented I think it's ok.

[chancancode]

The feature was added in #16399, but we still need to add the tests, update the docs and add a CHANGELOG (see #15592 (comment)).

Ideally, we should refactor the test similar to how we test {{#each}}, so we can run all the same test cases against { foo: "bar", ... }, EmberObject.create({ foo: "bar", ... }), Object.assign(Object.create(null), { foo: "bar", ... }), new Map([["foo", "bar"], ...]), new CustomIterable({ foo: "bar", ... })`, etc.

If you couldn't figure out a good way to do that refactor, we can land the test you have now and I can refactor them later also.

[chancancode]

What we do in the #each tests is that we made up an abstraction/API for creating and manipulating arrays and make all the tests go through those APIs instead of modifying the array directly. That way, we can easily run all the same test cases against multiple type of array-like objects (now I realized we should probably add Set there now that we added support).

[cibernox]

Nice. I'm AFK all weekend but I can check it next week