ref: prefer for..of loops over indexed access

[TkDodo]

This PR refactors traditional for loops which use an indexed access to for..of loops, if the index was only used to access the item of the collection that was iterated over.

for..of loops plays better with noUncheckedIndexedAccess, as they avoids a no-null-assertion (!)

The prefer-for-of rule of typescript-eslint was also enabled.

Note: These changes should be mostly safe. There’s a slim chance that we were looping over a non-iterable, like an object that had a length property. In one case, the types disallowed for..of because the element is not an iterable, only an indexable. Iterating over a non-iterable will yield a runtime error.

IF we had something that was any on type level, or was asserted to be an array, when it was in fact an indexable at runtime, we would now get an error with for..of.

[gggritso]

🧹

[natemoo-re]

Nice cleanup! 🥔🥔🥔

[JonasBa]

We need keep profiling to still using the old for loop as that is very performance sensitive part of the codebase (I can fix it after the merge too). The reason is that for of loops have extra iterator overhead that might end up negatively impacting the flamegraph import, stack iteration and a lot of our rendering code which can easily run in the magnitude of 100k loops per second across the render pipeline and with the addition of continuous profiling, the likeliness that we'll end up working with even more in the future is even higher :/

I would also say that I'm generally not entirely on board with this (my philosophy is to just let people write code, as long it's safe) and I think this change might result in a slightly more subtle debugging experience at the expense of convenience.

For example, if you consider a user that is investigating a cannot read property x of undefined error for the following two cases, the good ol for loop nudges towards more runtime safety and the inconvenience of writing the check can be seen as a feature :)

for(let i = 0; ...){
  const item = arr[i]! // <- unsafe
  doSomething(item)
}

vs

for(item of arr){
  doSomething(item)
}

That said, the nice side benefit of this is that we will be warned about non iterables, which I think is a good improvement. I wanted to highlight the small change, but imo my concern isn't strong enough to warrant blocking this.