Skip to content

[Arrays] To convert an array-like object to an array, use Array.from #1084

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
rileybracken opened this issue Sep 19, 2016 · 7 comments
Closed

Comments

@rileybracken
Copy link

rileybracken commented Sep 19, 2016

In 4.3 you say to use the array operator to copy arrays.

Why not do the same for creating arrays 4.4?

const foo = document.querySelectorAll('.foo');
const nodes = [ ...foo ];

Obviously the title would need to change, but I think the syntax is more consistent with 4.3.

@ljharb
Copy link
Collaborator

ljharb commented Sep 19, 2016

That's a good point - we should be recommending the spread operator here, even though they're equivalent.

It might also be worth noting that it could be helpful to avoid the array creation step in some cases, ie:

// bad
const bar = [...foo].map(mapper);

// good
const bar = Array.prototype.map.call(foo, mapper);

@kesne
Copy link
Contributor

kesne commented Sep 19, 2016

Why would the array creation be a problem? IMO Array.prototype.map.call(foo) is harder to reason about than [...foo].

@ljharb
Copy link
Collaborator

ljharb commented Sep 19, 2016

I agree with you a little bit - but the efficiency of avoiding the extra array reification matters. It's why Array.from takes a callback - and now that I think of it, that's why Array.from(foo, mapper) is actually loads better than [...foo].map(mapper).

@kesne
Copy link
Contributor

kesne commented Sep 19, 2016

A wise man once told me that the last thing we should care about is performance.

@rileybracken
Copy link
Author

rileybracken commented Sep 19, 2016

I might be wrong on this but I just did a speed test on both and the spread test keeps coming on top.

const arr = [ ...Array(100000).keys() ];
let test = function (i) { return i + 1; };

let arrFrom = function () {
  console.time('arrayFromTest');
  Array.from(arr, test);
  console.timeEnd('arrayFromTest');
};

let arrSpread = function () {
  console.time('arraySpreadTest');
  [...arr].map(test);
  console.timeEnd('arraySpreadTest');
};

arrFrom();
arrSpread();

What am I doing wrong?

results:
image

@ljharb
Copy link
Collaborator

ljharb commented Sep 19, 2016

@rileybracken "running a microbenchmark". Performance can't be usefully measured in that way, since the engine will optimize the mapper function. Try using eval in the function, for example, to ensure it's never optimized.

@rileybracken
Copy link
Author

Ah, now results are all over, I think I will just have to believe you 😉.

Here it is if you want to try it. http://jsbin.com/garayo/1/edit?js,console

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants