Skip to content
This repository has been archived by the owner on Aug 28, 2020. It is now read-only.

reactivity - each blocks - workaround to update array with primitives values #15

Closed
opensas opened this issue Apr 28, 2020 · 11 comments
Closed
Assignees
Labels
question Further information is requested

Comments

@opensas
Copy link
Contributor

opensas commented Apr 28, 2020

I'm reading the section about reactivity and the way svelte handles it inside each blocks.

this is the example to show how svelte does NOT update the referenced array when items are primitive values:

<script>
  let list = [1, 2, 3];
</script>

{#each list as item}
  <button on:click={() => item *= 2 }>{ item }</button>
{/each}

this is a possible workaround that could be added to the recipe: (check the repl)

<script>
  let list = [1, 2, 3];
  const updateList = (index) => list[index] *=2
</script>

{#each list as item, index}
  <button on:click={() => updateList(index) }>{ item }</button>
{/each}

Moreover, doing some tests I found a strange beahviour regarding svelte and arrays with primitive values. If I try to assign an item which is a primitive value from inside an each block, just like the example above, the array reactivity is broken, it seems like it changes the array reference to something else.

Here's is a repl reproducing this behaviour. As soon as you click one button of the first group, the array will no longer be updated.

I guess that svelte should try to issue a warning when this happens, updating an item which is a primitive value from an each block should not be allowed if it carries this nasty side effect. Do you think I should create an issue at svelte for such a thing?

BTW: great initiative this recipes repo, it's really bery useful, and the reactivity section was very much needed indeed.

@opensas opensas added the question Further information is requested label Apr 28, 2020
@swyxio
Copy link
Member

swyxio commented Apr 28, 2020

nice! i think @pngwn was the one who wrote that?

idk if this is a bug - svelte specifically warns you that you need to treat arrays as immutable. maybe we can error or handle this usecase better.. maybe this is something you should file on the Svelte repo itself?

@pngwn
Copy link
Member

pngwn commented Apr 28, 2020

Hmm, this was true when I wrote it for sure because I remember having a lengthy discussion about it with rich and conduitry. I’ll check this behaviour again and see if it is a bug or not.

@pngwn
Copy link
Member

pngwn commented Apr 28, 2020

I have totally misinterpreted this entire conversation leading to much confusion and history searching on my part.

To the point, I think that the simplest (and most like-for-like) workaround for this is to modify the array, by index, directly in the each block rather than using a helper:

<script>
  let list = [1, 2, 3];
</script>

{#each list as item, i}
  <button on:click={() => list[i] *= 2 }>{ item }</button>
{/each}

@pngwn
Copy link
Member

pngwn commented Apr 28, 2020

Regarding the error, feel free to file a bug/ enhancement on the svelte repo. I think a runtime dev warning would be good in this case but there has been some broader discussion about dev warnings in general and how far we should go with them. Not sure there is consensus on that issue as of yet.

@opensas
Copy link
Contributor Author

opensas commented Apr 28, 2020

You are absolutely right, using the index in the each block itself is far easier, I overcomplicated things, I hope you can add it to the recipe, it's a really clean and easy workaround.
I will propose the warning (maybe even an error), svelte shouldn't allow that kind of things, specially if it breaks the reactivity on the whole array, that can not possible be a desired behaviour on part of the dev

kevmodrome added a commit that referenced this issue Apr 29, 2020
Add pngwn workaround to example - refs #15
@swyxio
Copy link
Member

swyxio commented Apr 29, 2020

@opensas arrays are tricky with svelte :( i ran into this the very first day i used svelte. took a while to get over it. hope u filed that svelte issue, see what they say.

@opensas
Copy link
Contributor Author

opensas commented Apr 30, 2020

yeap, I created this issue

and yes, sometimes I'm not quite sure what will svelte track when I change an array's item, or if it will re-render the whole thing or just the items that changes, and I do some quick test to find out.
It would be great to add a dealing with arrays section in the recipes explaining these things

@swyxio
Copy link
Member

swyxio commented Apr 30, 2020

be my guest :) pr welcome

@tanhauhau
Copy link
Collaborator

@opensas this has been fixed in 3.23.1

sveltejs/svelte#4744 (comment)

@opensas
Copy link
Contributor Author

opensas commented Jun 9, 2020

added pr 37 to state that in the recipe

@opensas
Copy link
Contributor Author

opensas commented Jun 11, 2020

The workaround has been added to: https://github.com/svelte-society/recipes-mvp/blob/master/language.md#each-blocks

Moreover, Svelte fixed this in version 3.23.1 in this issue

so I'm closing this issue

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants