Skip to content
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

#each: Iterate Object Properties #894

Closed
ChrisTalman opened this issue Oct 12, 2017 · 23 comments
Closed

#each: Iterate Object Properties #894

ChrisTalman opened this issue Oct 12, 2017 · 23 comments

Comments

@ChrisTalman
Copy link

It would be useful for the #each block to be capable of iterating the properties of an object.

For instance, the following object.

{a: 1, b: 2, c: 3}

I attempted this here, but it seems to fail.

@Conduitry
Copy link
Member

Yeah, this isn't supported currently. I'm not sure what the plans are for it. If it were to be implemented, I think it'd be with a different syntax, so that Svelte could determine at compile time whether this is array or object iteration. (We don't want to output extra code that has to be run all the time to check whether the data to iterate over was an array.)

I think what's been suggested in the past is to use computed properties with Object.keys/Object.values/Object.entries/etc. The question is whether something explicit like that is worse than some new syntax specifically for object iteration.

Also, if Svelte supported this, it would have to be subjected to all the niggling iteration order concerns in different browsers, especially older ones. If we force user code to provide an array, it's very clear what Svelte should be doing.

@m59peacemaker
Copy link
Contributor

I agree with destructuring like #888 rather than having new svelte syntax.

@Conduitry
Copy link
Member

With the new destructuring feature, here's how this could be done explicitly with Object.entries. This seems to mostly work, but if you open the console and run component.set({things:{foo:'bar'}}), what's displayed is (a: 1) instead of (foo: bar). It looks like the computed things_entries value is correct, and I haven't dug more into what's going on there.

@Conduitry
Copy link
Member

This should be good now in 1.41.1, although the website (and REPL) is currently down.

@Conduitry
Copy link
Member

All right, you can check my example out in the REPL now with the new version. @ChrisTalman What do you think of a solution like this using destructuring?

@ChrisTalman
Copy link
Author

Thanks for looking into this @Conduitry. This is certainly a viable solution, although the pure experience of simply providing an object with the existing syntax with no additional legwork (i.e. the computed property and the destructuring syntax) would be more elegant.

If I understand your previous comments correctly, is the concern that type checking will be expensive for performance, and that ordering might be inconsistent?

@Conduitry
Copy link
Member

@ChrisTalman Yeah, supporting this with the same syntax would mean extra code for all {{#each}} blocks, regardless of whether the component author needs to be able to iterate over both arrays and objects - so I'm basically positive that that route would be right out.

The concern about consistent ordering is present whether the same or a separate dedicated syntax is used. But there are also other issues of there being no single obviously correct thing for Svelte to do in this case. Do we use Object.entries, which is not supported everywhere? Do we use for-in? What about enumerable properties / own properties / etc? IMO it's better to push all of those decisions onto the component author, who hopefully knows exactly what they want, and let them specify this explicitly.

@ChrisTalman
Copy link
Author

@Conduitry Thanks for your thoughts.

The ordering problem is challenging. On the one hand, you could simply admit that ordering may not be consistent, which is essentially an inherent reality of JavaScript. So, if developers want to ensure consistency, they would need to use an array in any case. Alternatively, the different methods of ordering could be exposed in some way through some Svelte syntax, but that might not be worth the effort. I can certainly see why handing it off to the developer is desirable, especially given that this is not a major inconvenience.

Could you please explain the 'extra code' problem? I'm not familiar with why this is an issue.

@Rich-Harris
Copy link
Member

Could you please explain the 'extra code' problem? I'm not familiar with why this is an issue.

If you take some code like the default each block example...

<h1>Cats of YouTube</h1>

<ul>
  {{#each cats as cat}}
    <li><a target='_blank' href='{{cat.video}}'>{{cat.name}}</a></li>
  {{/each}}
</ul>

...Svelte iterates over the cats array (when the block is created/updated) using a for loop:

for (var i = 0; i < cats.length; i += 1) {
  each_blocks[i] = create_each_block(state, cats, cats[i], i, component);
}

If it didn't know if it was an array, or an object, or some other type of iterable, it would have to do this sort of thing instead:

if (Array.isArray(cats)) {
  for (var i = 0; i < cats.length; i += 1) {
    each_blocks[i] = create_each_block(state, cats, cats[i], i, component);
  }
} else {
  for (var key in cats) {
    each_blocks.push(create_each_block(state, cats, cats[key], key, component);
  }
}

Or if you assumed an ES6 environment (which Svelte currently doesn't) and wanted to support iterables, you could do

if (Symbol.iterator in cats) {
  let i = 0;
  for (const cat of cats) {
    each_blocks[i] = create_each_block(state, cats, cat, i++, component);
  }
} else {
  for (const key in cats) {
    each_blocks.push(create_each_block(state, cats, cats[key], key, component);
  }
}

But then that's slower than iterating over an array, so you might want to have one case for arrays, another for non-array-like iterables, and another for non-iterable objects.

That could be abstracted out into a helper, which would lessen the impact on apps that were compiling with shared: true...

// in helpers
export function each(things, callback) {
  var i = 0;
  if (Array.isArray(things)) {
    for (; i < things.length; i += 1) {
      callback(things[i], i, i);
    }
  } else if (typeof Symbol !== 'undefined' && Symbol.iterator in things) {
    for (const thing of things) {
      callback(thing, i, i++);
    }
  } else {
    for (const key in things) {
      callback(things[key], key, i++);
    }
  }
}

// in compiled code
each(cats, function (cat, key, i) {
  each_blocks[i] = create_each_block(state, cats, cat, key, component);
});

...but it's still more code, and users of standalone components would suffer. And there's still lots of nuance to deal with around situations like this:

component.set({
  things: { foo: 1, bar: 2, baz: 3 }
});

// should the each-block re-order? Or should the key for the first iteration
// change from 'foo' to 'baz', and so on? (Which would require more code)
component.set({
  things: { baz: 4, bar: 5, foo: 6 }
});

Oh and you might want special handling for Map objects, depending on whether this...

{{#each map as value, key}}

...feels more natural than this:

{{#each map as [key, value], i}}

This would add (modestly) to the convenience of developers who need to enumerate over objects, who I would wager are in the minority. For everyone else, it adds overhead and ambiguity. Leaving the decision in the hands of the developer means no-one is left confused about what's going on.

@tomcon
Copy link

tomcon commented Nov 27, 2017

Have found this functionality useful in the past. Couple of things:

  1. Can we not do something simple as this:

iterate list, use "as" keyword

{{#each cats as cat}} 

iterate object, use "in" keyword

{{#each prop in cat}}
  1. Also, re. "extra code" - as svelte runs at build time I don't see a problem.
    (then again, I know I don't see as much as others : )

@Rich-Harris
Copy link
Member

{{#each prop in cat}} is an interesting idea Though at the moment the order is reversed — things as thing instead of thing in things — so it might be a tiny bit confusing.

Maybe we should have a for:

{{#for key in object}}
  {{object[key]}}
{{/for}}

{{#for thing of iterable}}
  {{thing}}
{{/for}}

{{#for [key, value] of map}}
  {{key}}: {{value}}
{{/for}}

Not an ideal solution for plain objects though, since you need to do object[key] to get the value. Just throwing it out there. I think my preference is still for the simplicity of each.

Also, re. "extra code" - as svelte runs at build time I don't see a problem.

It means that the compiler would have to generate extra code for all components whenever it can't know what kind of thing it's going to be dealing with at runtime — not that it's extra code inside the compiler itself.

@Rich-Harris
Copy link
Member

Going to close this as it adds way too much complexity and overhead given for something you can already do trivially with each Object.entries(obj) and each [...iterable]. Better to have one clear way of doing things rather than multiplying the stuff that needs to be maintained and documented.

@buckle2000
Copy link

buckle2000 commented Oct 17, 2019

Is there a reason that {#each} can't accept any iterable? [...iterable] is rather hard to read (e.g. [...seq.filter(f).take(5)] if I use https://immutable-js.github.io/immutable-js/docs/#/Seq). Must svelte know the length of the array to update DOM?

@pngwn
Copy link
Member

pngwn commented Oct 17, 2019

The current implementation uses the length property of the array to be iterated.

@marcus-sa
Copy link

marcus-sa commented Oct 17, 2019

@pngwn converting an iterable to an array will be a lot slower than just iterating over it, plus it doesn't really makes sense as for...of can be used for exactly this.

@pngwn
Copy link
Member

pngwn commented Oct 17, 2019

I'm not sure why this was directed at me, I was simply answering a question. I'm familiar with how the language works.

@samuelgozi
Copy link
Contributor

samuelgozi commented Nov 26, 2019

Why not use internally Array.from when available?
The code can be much simpler:

  cats = Array.from ? Array.from(cats) : cats;
  for (var i = 0; i < cats.length; i += 1) {
    each_blocks[i] = create_each_block(state, cats, cats[i], i, component);
  }

The upsides of using this method are that it doesn't require any API changes, It's backward compatible and that it supports maps and sets by default.

The only downside is that maybe this method adds overhead when looping through regular arrays but its mostly implementation-dependent.

EDIT: Performance doesn't seem to be impacted that much, you can see the results here: https://jsperf.com/array-from-middleware

@robinvandernoord
Copy link

I don't know if this is still relevant, or an update has been released since, but the best way I could find to do this, is like:

    {#each Object.entries(cats) as [cat_name, cat_number]}
        {cat_name}: {cat_number}
    {/each}

I saw Object.entries mentioned in this thread before, but did not see any code sample of it, so I thought it could be handy for someone if I posted this.
Hope it helps anyone!
P.S. the [] braces are required, because otherwise cat_number will be the index of the key. If you need the index as for example an iterator or count, you can deconstruct the object like:

as [cat_name, cat_number], index

@mirko
Copy link

mirko commented Sep 15, 2020

There's a problem with the last answer: e.g. animations do not work.
Doing e.g.:

    {#each Object.entries(cats) as [cat_name, cat_number]}
        <p in:receive="{{key:cat_name}}" animate:flip>

throws
[!] (plugin svelte) ValidationError: An element that use the animate directive must be the immediate child of a keyed each block

@tanhauhau
Copy link
Member

@mirko could you provide more context from your code, preferably in a svelte.dev/repl, so we can figure out why?

@robinvandernoord
Copy link

@tanhauhau I assume because of the unpacking in [cat_name, cat_number], svelte can't handle it? Since it then isn't an immediate child of a keyed each block anymore. No idea how we can properly fix that.

@mirko
Copy link

mirko commented Sep 20, 2020

I'm sorry, I commented on this too soon. I wasn't aware one could also provide a key to an array returned per iteration rather than a single value.
What works:
{#each Object.entries(cats) as [cat_key, cat_val] (cat_key) }
(/cc @tanhauhau @robinvandernoord )

@effulgentsia
Copy link

For what it's worth, I think the following syntax sugar would be nice:

<!-- 
Same as {#each Object.entries(cats) as [cat_key, cat_val]}
-->
{#each cats as cat_key:cat_val}

<!-- 
Same as {#each Object.values(cats) as cat_val}
-->
{#each cats as :cat_val}

I'm considering writing a Svelte preprocessor that implements the above (converts the shorter syntax into the longer syntax), but I haven't written a Svelte preprocessor before so I don't know when I'll get to it or how long it will take so wanted to share the idea in the meantime in case anyone else wants to run with it or if Svelte maintainers want something like that in Svelte itself.

sacrosanctic pushed a commit to sacrosanctic/svelte that referenced this issue Dec 24, 2024
Fix incorrect value in <select> element.

Co-authored-by: 15363205631 <q.2@foxmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests