Add spread rule for iterable objects#264
Conversation
|
not seeing much of a difference in safari, although generally i tend to prefer functions over language syntax due to flexibility and composability |
|
yeah looks like Safari is the only one marginally slower, but both desktop and mobile which applies to web views so Chrome on iOS |
|
I could mostly go either way on this one, personally. I think I visually prefer |
|
I also personally prefer I definitely prefer |
|
I like the spread syntax a lot, but I don't like the inconsistency in using both spread and Since That being said, if cases where spread doesn't work are fairly rare for us then I'd prefer using spread |
| ``` | ||
|
|
||
| - [8.4](#8.4) <a name="8.4"></a> Use the spread operator (`...`) to copy arrays, rather than iterating over the array or using `Array#slice`. If you need subsections of the array, continue to use `Array#slice`. | ||
| - [8.4](#8.4) <a name="8.4"></a> Use the spread syntax (`...`) to copy arrays, rather than iterating over the array or using `Array#slice`. If you need subsections of the array, continue to use `Array#slice`. |
lemonmade
left a comment
There was a problem hiding this comment.
I'm cool with this 👍 can we also add an issue to make this into a linting rule?
| ``` | ||
|
|
||
| - [8.5](#8.5) <a name="8.5"></a> To convert from an array-like object to an array (for example, a `NodeList` returned by `document.querySelectorAll`, or a jQuery object), use `Array.from`. | ||
| - [8.5](#8.5) <a name="8.5"></a> To convert from an array-like object to an array use `Array.from`. |
There was a problem hiding this comment.
Can we just ditch this? I don't think it's particularly relevant advice and it makes the rule feel harder to understand than it really is (just use spread)
There was a problem hiding this comment.
I'm in favor of ditching this, although spread throws in this example because it has no iteration protocol, e.g.
const arrayLikeObject = {
0: 'a',
1: 'b',
length: 2,
[Symbol.iterator]() {
let index = 0;
return {
next: () => {
return this[index]
? {value: this[index++], done: false}
: {done: true};
},
};
},
};|
I much prefer I think the only case I'd favor spread over const newArray= [
...nodeList,
el,
...otherArrayLike,
];Personally, I think spread is for de-structuring and joining arrays, not converting things into them |
370cc43 to
df739a3
Compare
df739a3 to
504c812
Compare
|
I removed the
And opened #265 to add a linting rule |
Reasons:
NodeList, a test with jQuery object, and a chart) but we are talking high operations a second so not sure it is a noticeable differenceAlso updated mentions of spread operator to spread syntax, as it is not an operator.
@Shopify/javascript