Array.from doesn't support iterables

[jods4]

Before the recent removal of corejs polyfills I had this in my code:

let s = new Set();
s.add(1);
s.add(2);
let a = Array.from(s.values());

And it worked (as it should).

Now it doesn't work anymore.

A brief look at aurelia-polyfill makes me think that Array.from only supports array-like parameters (with length and an indexer). Per the specs it must also support iterables (objects with a next() function that returns { value, done }).

This is a breaking change.

[EisenbergEffect]

We're happy to accept a pull request to fix that and would release it asap once provided.

[jods4]

In the end I added back corejs to my project. We have a large, complex LOB application and it was written from the scratch with the assumption that we could assume ES6 level of features (although not extensively). After the last update, many things started to break: for of loops because Symbol polyfill was missing (we use Babel to transpile). Missing Promise polyfill. Array.from(new Set().values()) was not properly supported in Aurelia. string.repeat() was missing.

So I made the call that the risk of regression was just too big and not worth it. We put back a custom build of core-js that only contains ES6 polyfills. It weights around 60Kb minified (not gzipped). For an intranet app it's acceptable, especially given the full app size and the fact that after first loading it will stay in browser cache.

Now the question for me is rather: is there an easy way to remove aurelia-polyfills? It seems like an unrequired dependency now.

I created a PR for you anyway: #5. I think that it makes no sense to expose Set and Map polyfills and not properly support Array.from on the results of .values() and .keys().

The PR seems pretty straightforward to me, but please note that because of above rollback to corejs I did not test this exhaustively. I did not find a test suite, otherwise I would have added a few tests along the lines of:

var s = new Set();
s.add(1); s.add(2); s.add(3);
expect(Array.from(s.values()).join(",")).toBe("1,2,3");

[EisenbergEffect]

We've added a Symbol polyfill as well as Symbol.iterator (for/of) support for all collections. We've fixed up Object.assign to work with Symbols as well.

[jods4]

My PR should be modified then. In my PR, because there was no Symbol.iterator I looked at the next() method directly.

It would be cleaner (and more specs compliant) to modify it so that:

• Array.from first checks for @@iterator. If there is one it should do the next() loop on [Symbol.iterator](), which actually just returns this but serves as a marker interface (more or less).
• I don't close the iterator correctly. On arbitrary operators there might be a return() method.
• Otherwise Array.from should fall back on the array-like code.

At this point, I think with Symbol.iterator built-in aurelia-polyfills it would be best to grab a correct Array.from polyfill from existing sources such as CoreJS. My PR was merely a work-around to make Array.from(set.keys()) & co. work (as they were also in the included polyfills) but it is not completely spec-compliant.

[EisenbergEffect]

Would you be interested to locate such a polyfill or "port" one. We would like something without any external dependencies beyond what we already have in our polyfills library.

[jods4]

I updated my PR by taking the CoreJS implementation (mostly). Because CoreJS is made of tons of tiny reusable parts I had to merge stuff and simplify a little bit the paranoid checks that litter this code.

Beware that I did this directly in Github during a commute... so I did not run this code even once! Maybe it does not work. It seems to me it should be OK though and hopefully it will help you get a better Array.from more quickly.

Sorry about that but as I'm using CoreJS again at work it doesn't leave me a lot of time to test alternative polyfills...

[EisenbergEffect]

Understood. Thanks for the assistance!

[EisenbergEffect]

The main thing we wanted was to remove the hard dependency that we had on core-js which left developers without options. We wanted to also have our own solution and open up choices for developers. Thanks for helping!

[jods4]

I'm afraid you've opened Pandora's box, though... now people (like me?) will find edge cases that don't work (I saw something about Object.keys(null)) and you'll need to maintain your own polyfill stuff :(

Why didn't you use the custom build option of CoreJS to get just the bits you needed?

Also, a good thing would be to put aurelia-polyfills as a totally optional dependency that needs to be required first. So that people really have the choice to use another (e.g. corejs), use none at all (if target platform has enough support -- ES6 browsers are practically here now), or use aurelia-polyfill.

[EisenbergEffect]

The core-js project is in danger, so we don't want any dependency on it in our core.

It's possible to avoid our own polyfills by writing your own bootstrapper, which isn't hard, or manually bootstrapping the framework in code. But, I'm not sure it's worth it for most people, especially since our polyfills are pretty small.

[jods4]

It's true that they are small. Any code example? We already bootstrap Aurelia in code (if I understand you correctly).

Just out of curiosity could you quickly clue me in as to why core-js is in danger? Thanks!

[gheoan]

@jods4 I think Rob is referencing to this issue: https://github.com/zloirock/core-js/issues/139.

[EisenbergEffect]

The bootstrapper module is just a small bit of "startup" code that gets things working. It could be replaced with your own code. It's the only thing that references our polyfills, so that could be removed. Once we all move to jspm 0.17, the bootstrapper will get even simpler...making it easier to write your own if needed.

@Gheoan is referencing the correct issue. It's best for us to have as few hard dependencies as possible. So, even if core-js is ok, we all feel better not "forcing" developers to use it and not tying ourselves to things we can't control.

[jods4]

@Gheoan thanks for the pointer.

I feel like polyfills should never be a hard-dependency anyway. They are an implementation of a standard spec, you should be able to swap them at any point. As long as you know (and document) what features you depend on, you should be good.

We use AMD/requirejs here. When this project started JSPM seemed too "fresh" for us to place a confident bet on it. We had a lot of experience and past success with AMD, so that felt like a better choice.

[EisenbergEffect]

Awesome! I'd like to hear about how require.js has worked out. Please send me some email rob at durandal dot io I've used require.js a lot in the past. That's what Durandal was based on. I'd like to get some feedback from you on using it with Aurelia. Maybe we can make it easier. We could even do a custom bootstrapper if needed, like we just added for Webpack.

[AdamWillden]

I'd just like to register my support for this being part of aurelia-polyfills. I too have had to revert to core-js but if this were included in aurelia-polyfills I could lose my dependency on core-js.

Rob, I'm happy to see you've been receptive to its inclusion despite not being needed by aurelia itself. I think these polyfills cover the majority of commonly required cases. This is the last piece of the puzzle that needs filling for me :+1:

[EisenbergEffect]

Yes, we can add it. We just need to test the new version submitted based on iterables.

[AdamWillden]

@EisenbergEffect you can close this issue now :-)

When do you plan to do a release containing the PR? I was about to update and realised it's not included in a release yet.

[EisenbergEffect]

Coming tomorrow most likely.

[AStoker]

Sorry to resurrect this one from the dead. We're running into issues using the polyfills and Array.from with Maps, and specifically when we use a map function. The error we get running this code is:

var map = new Map();
map.set('foo', 1);
Array.from(map, function(x){
  return x[1] = x[1] + 1
});

TypeError: fn is not a function. (In 'fn(a1, a2)', 'fn' is an instance of Array)

This is coming from this line https://github.com/aurelia/polyfills/blob/master/src/array.js#L11

Can anyone else confirm? Is this a known issue? It slipped by us for a while because Array.from is implemented in ES6 and almost all major browsers. IE doesn't natively support Array.from, and that's where we've found the issue.

[jods4]

As I noted in the PR, the problems seems to be that L32: https://github.com/aurelia/polyfills/blob/master/src/array.js#L32 should be result[index] = mapping ? iterCall(iterator, mapfn, step.value, index) : step.value; instead of result[index] = mapping ? iterCall(mapfn, step.value, index) : step.value;