svelte: Allow `#each` block to handle arbitrary non array like iterables
Describe the problem
Consider the following REPL:
<script>
const myMap = new Map();
myMap.set('key1', 'value1');
myMap.set('key2', 'value2');
myMap.set('key3', 'value3');
for(const [k, v] of myMap.entries()){
// Javascript lets us iterate through a set so I don't see why svelte shouldn't
console.log(k, v);
}
</script>
<!-- This is not allowed because mySet does not have `.length` -->
{#each myMap.entries() as [k, v]}
<h1>Key: {k}, value: {v}</h1>
{/each}
Obviously one could trivially achieve this by wrapping myMap in Array.from
or spreading into an array but it would be nice if svelte could handle iterables in addition to ArrayLike
objects.
Describe the proposed solution
The problem is caused by the fact that svelte generates the code which loops over the each value using its .length
property. This can be seen below:
// from top of create_fragment
let each_1_anchor;
let each_value = /*myMap*/ ctx[0].entries;
let each_blocks = [];
for (let i = 0; i < each_value.length; i += 1) {
each_blocks[i] = create_each_block(get_each_context(ctx, each_value, i));
}
and
// from the p function returned by create_fragment
for (i = 0; i < each_value.length; i += 1) {
const child_ctx = get_each_context(ctx, each_value, i);
if (each_blocks[i]) {
each_blocks[i].p(child_ctx, dirty);
} else {
each_blocks[i] = create_each_block(child_ctx);
each_blocks[i].c();
each_blocks[i].m(each_1_anchor.parentNode, each_1_anchor);
}
}
As you can see it assumes that each_value
(in this case myMap
) has a length
property. It seems to me that we could easily generate the following code which is equivalent:
// the top of create_fragment
let each_1_anchor;
let each_value = /*myMap*/ ctx[0].entries();
let each_blocks = [];
for (const item of each_value) {
each_blocks.append(create_each_block(get_each_context(ctx, item)));
}
// the loop in the p function generated by create_fragment
let each_value_i = 0;
for (const item of each_value) {
const child_ctx = get_each_context(ctx, item);
if (each_blocks[each_value_i]) {
each_blocks[each_value_i].p(child_ctx, dirty);
} else {
each_blocks[each_value_i] = create_each_block(child_ctx);
each_blocks[each_value_i].c();
each_blocks[i].m(each_1_anchor.parentNode, each_1_anchor);
}
}
With get_each_context
rewritten to be:
function get_each_context(ctx, item) {
const child_ctx = ctx.slice();
child_ctx[1] = item;
return child_ctx;
}
It’s worth noting that the for...of
syntax isn’t supported by IE11 but the svelte compiler appears to recommend the use of the spread operator which also isn’t supported in IE11 so perhaps this isn’t an issue.
Alternatives considered
The alternative here is to leave it as it is and tell people to use Array.from
and or [...mySet]
. It isn’t the end of the world.
But given that it seems so trivial to support arbitrary iterables and that javascript can loop over iterables I can’t see why svelte wouldn’t implement this. Is it because you want any infinite loops to be in user code rather than generated code?
Importance
nice to have
About this issue
- Original URL
- State: closed
- Created 2 years ago
- Reactions: 25
- Comments: 25 (18 by maintainers)
Commits related to this issue
- feat: allow #each to iteratate over iterables closes #7425 Uses a new ensure_array_like function to use Array.from in case the variable doesn't have a length property ('length' in 'some string' fails... — committed to sveltejs/svelte by dummdidumm a year ago
- feat: allow #each to iterate over iterables (#8626) closes #7425 Uses a new ensure_array_like function to use Array.from in case the variable doesn't have a length property ('length' in 'some string... — committed to sveltejs/svelte by dummdidumm a year ago
Doesn’t that defeat the performance benefit of Set and Map, though, when dealing with massive data sets?
In native JS, we can iterate over sets and maps using for…of and retain those perf benefits without needing to port their contents to an array. It would be ideal to keep those benefits.
I ran some benchmarks just to demonstrate this difference, and Map iteration is faster across all (by over 50%):
for..of(Map)
for..of(Array.from(Map))
for..of(Array.from(Map.entries()))
The fact alone that taking advantage of the perf benefits is so attainable and not something other frameworks have done yet is a huge opportunity for Svelte, and I think we’d be falling short to not take a key stake in big (and attainable) performance wins like this.
I think
for..of
loops require an iterable anyway, so by swapping to afor..of
loop here, you’d naturally be supporting both Maps and Sets and other iterable data structures.Implemented in https://github.com/sveltejs/svelte/pull/8626
Would love to see this added to the roadmap for v4
i cant overstate how exciting this is. this is the one problem that forces me to patch svelte at the moment. basically i have an arraylike js proxy object that unfortunately returns typeof as “function” for unrelated reasons
at the moment the each statement in svelte checks if something is an object which is not true and its also not clear why only objects with a length attribute should be allowed, instead arg?.length seems like reasonable check for array like.
currently the check is
additionally supporting generic iterables would solve a big pain for me as i often iterate of entries where the number is not known before the iteration ends, this forces me to guess the length and then run into all kinds of issues when when its off
@joeally Exactly. The Map and Set are not special cases. Every iterable can be iterated over using the same
for..of
loop style. The only exception is the “array-like” case, which is easy enough to support performantly, and one additional but optional exception is the plain array type.Array types are iterable in the same way that Map, Set, and generators are, but they actually iterate more performantly if iterated over using a plain
for
loop, so I use a!Array.isArray(obj)
check to push array types into the second block alongside the array-like type.We can loop over
someMap.entries()
andsomeSet.values()
alternatively, but generally speaking, it’s actually more performant to just loop over maps and sets directly without usingentries()
andvalues()
, which is also simpler syntax, so win-win. 🎉Not sure why I’m not seeing @eabald’s comment here anymore, so reposting it here. As it echos the same comments many of us have voiced before:
I think maps and sets both really need this. This is a massive performance deficit for Svelte and can easily be avoided as demonstrated earlier and still support an index, so I’d also argue that the lack of a natural iterable index is a weak argument as well.
Also, for many iterables, like maps and sets, you can use
size
instead oflength
and get the expected results.The helper function I outlined above is a single function that accounts for this and manages all cases. With a small tweak, you could limit the performance boost to maps and sets for their
size
property and convert everything else to an array.Were there any other downsides to using that?
As @lucidNTR mentioned, users can also do that conversion to arrays explicitly if it’s what they want, but in most cases, I explicitly work with maps and sets because they’re generally more performant with large datasets. The current implementation flips that upside down and is slower than using an array to begin with.
@dummdidumm by that argument you could just say people should wrap iterables with array.from in application layer, that way there would be 0 lines of code change in svelte and we would not break expectations. supporting iterables means by definition allowing to start rendering pages of entries from the iterable without accessing length, if this is too much work at the moment this should just be postponed but this will lead to many issues
@dummdidumm Thanks for all your work on this!
Did you see my earlier comments about iterating over iterable values directly (maps, sets, generators, etc.), which greatly improves performance?
If I’m reading the merged PR accurately, it looks like those would all now be converted to arrays using
Array.from
, which is significantly slower.Amazing to see this change in Svelte 4.0. Thanks for all of the hard work @dummdidumm and anyone else who worked on it.
@brandonmcconnell -
I’d prefer the user to have to call.entries()
. IMO the{#each}
should work with any iterable (see: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Iteration_protocols).{#each}
should not have a special case forMap
andSet
.Yes you’re right. I didn’t know that
Map
itself can be iterated over.I agree that a solution like
'length' in foo ? foo : Array.from(foo)
would be ideal, though I think we could still preserve the performance benefits of Maps and Sets with minimal effort.What “ability to do minimal DOM updates” would be lost by switching to use
for..of
loops?If we use a lightweight abstraction like the
iterateOver
function below, we can keep all the benefits you mentioned and also support all iterables (e.g. maps, sets, generators, arrays, etc.) and array-likes.Implementation
— see on TS Playground
In ☝🏼 this example, I am using an imaginary “expose” function, but that would just be replaced by whatever logic or helper function you use to slot in the HTML for each
{#each}
block iteration.Notes on why I used the
!Array.isArray(obj)
check to push array iterables to theelse
blockTests
— see all tests on TS Playground (same link as above)
Here are some tests for that function, each of which works as expected:
{ length: 3 }
['value1', 'value2', 'value3']
{'value1', 'value2', 'value3'}
{'key1' => 'value1', 'key2' => 'value2', 'key3' => 'value3'}
'value1' => 'value2' => 'value3'
The DOM is slow, and I am almost certain that losing the ability to do minimal DOM updates would far outweigh what we’d gain by only iterating through the array once with
for of
. Usingfor of
internally would also mean that we could no longer directly support iterating over the array-like object{ length: 42 }
, which is something we’ve encouraged people to do in the past when all they really care about is the number of times something appears in the DOM. We would probably want to have something like'length' in foo ? foo : Array.from(foo)
so we don’t lose performance for things that already are arrays or other array-likes.Rich Harris has recently talked about supporting this too: https://youtu.be/dB_YjuAMH3o?t=4m18s