Add toArray utility function to convert array-like objects to arrays. #3389
Conversation
| @@ -36,6 +36,22 @@ export function isArray(value) { | |||
| } | |||
|
|
|||
| /** | |||
| * Converts an array-like object to an array. | |||
| * @param {?HTMLCollection|?NodeList|*} arrayLike | |||
There was a problem hiding this comment.
For reference, Closure's doc comment.
There was a problem hiding this comment.
Thanks! Used the closure compiler IArrayLike type here.
|
I found out this |
|
Never mind, I think the polyfill for |
|
How do we know whether we need to polyfill or not? |
| expect(arr.length).to.equal(4); | ||
| expect(Array.isArray(arr)).to.be.true; | ||
| }); | ||
| }) |
|
@muxin PTAL. I opted not to do the |
|
LGTM |
| @@ -36,6 +36,26 @@ export function isArray(value) { | |||
| } | |||
|
|
|||
| /** | |||
| * Converts an array-like object to an array. | |||
| * @param {?IArrayLike<*>|string} arrayLike | |||
| * @return {!Array.<*>} | |||
There was a problem hiding this comment.
No .. Also, should we not be using the @template T?
There was a problem hiding this comment.
Not sure, we seem to be using Nevermind, I see we do use * everywhere, @cramforce @erwinmombay any idea?@template syntax.
| } | ||
| const length = arrayLike.length; | ||
| const array = new Array(length); | ||
| for (let i = 0; i < array.length; i++) { |
There was a problem hiding this comment.
arrayLike.length. No idea how the browser will optimize when you're "iterating" the object you're modifying.
There was a problem hiding this comment.
I am not modifying the length but updated to use arrayLike.length just in case
There was a problem hiding this comment.
I am not modifying the length
Right, but the JS engine may not know that.
|
Adding LGTM label - Yuxi already LGTM'd in comments. |
Separating this from a previous PR.
ITI: #3343