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

Svelte 5: Deeply nested state doesn't re-render when looping #10685

Closed
alex-way opened this issue Mar 3, 2024 · 17 comments · Fixed by #10699
Closed

Svelte 5: Deeply nested state doesn't re-render when looping #10685

alex-way opened this issue Mar 3, 2024 · 17 comments · Fixed by #10699
Assignees
Milestone

Comments

@alex-way
Copy link

alex-way commented Mar 3, 2024

Describe the bug

It seems that when looping through a deeply nested object, svelte doesn't re-render when looping through object.

No errors are thrown in the logs which makes me think this may be a bug.

Reproduction

Repl

Click the update button and observe that the JSON.stringify correctly shows the update, but the each block isn't updated with the new value.

Note: I'm using the $inspect to force the compiler to svelte 5 mode. Without it, it works as expected.

Logs

No response

System Info

Browser: Edge

Severity

blocking all usage of svelte

@alex-way
Copy link
Author

alex-way commented Mar 3, 2024

Maybe #9546 is related?

@brunnerh
Copy link
Member

brunnerh commented Mar 3, 2024

The #each loop variable in runes mode is no longer "connected" to the reactivity of the source array.

So unless you make the items themselves reactive, you need to use access via key on the source (which is less efficient as far as I am aware).

const state = $state(initialValue); // adds proxy reactivity to object
const roomState = writable(state);

(The statefulness would not survive complete re-assignment, so set/update probably would need to be wrapped to account for that. In Svelte 5 the use of the store should not be really necessary in the first place, though.)

REPL

{#each Object.values($roomState.users) as user (user.name)}
  {$roomState.users[user.name].value}
{/each}

REPL

@Prinzhorn
Copy link
Contributor

The #each loop variable in runes mode is no longer "connected" to the reactivity of the source array.

So unless you make the items themselves reactive, you need to use access via key on the source (which is less efficient as far as I am aware).

This can't be the expected behavior for stores, this is clearly a regression that needs to be fixed.

@brunnerh
Copy link
Member

brunnerh commented Mar 3, 2024

Some migration work can be expected when switching to runes mode, but I guess it would make sense to have special treatment for stores. Store usage is at least visible to the compiler via the $store syntax, so differentiating would be possible.

@dummdidumm dummdidumm added this to the 5.0 milestone Mar 3, 2024
@trueadm
Copy link
Contributor

trueadm commented Mar 4, 2024

If we were to put in special handling when we see $store syntax, then people who might use stores without that syntax might expect it to work the same. Furthermore, what if someone did?

<script>
  // ..
  const users = Object.values($roomState.users);
</script>
{#each users as user (user.name)}
  {user.value}
{/each}

Personally, this feels like a breaking change that we need to note and cover with migration notes. We can't really do a runtime check here either.

@dummdidumm
Copy link
Member

That snippet is static because it's not using $derived, so it would be expected to not be reactive in that case.

Can you elaborate why the change isn't noticed in this case? I haven't dug into it yet.

@trueadm
Copy link
Contributor

trueadm commented Mar 4, 2024

Sorry, I meant to make it @const instead.

@Prinzhorn
Copy link
Contributor

Prinzhorn commented Mar 4, 2024

If we were to put in special handling when we see $store syntax

That's the whole idea behind stores though? Every single expression that contains $store is invalidated, when the store changes.

@const also work perfectly fine in non-runes mode and breaks in runes mode, just like with the expression in #each

then people who might use stores without that syntax might expect it to work the same

What does "without that syntax" mean? The dollar prefix is the syntax.

@trueadm
Copy link
Contributor

trueadm commented Mar 4, 2024

What does "without that syntax" mean? The dollar prefix is the syntax.

It also works without the syntax in Svelte 4.

@Prinzhorn
Copy link
Contributor

the syntax

I still don't know what syntax you're talking about :D. I assume you mean regular reactivity?

Also this already works perfectly fine with $state

I don't understand the discussion. The expression Object.values(foo) needs to be invalidated when foo changes? That's how Svelte works.

@trueadm
Copy link
Contributor

trueadm commented Mar 4, 2024

The expression Object.values(foo) needs to be invalidated when foo changes?

Yes, but that logic can be in another function too.

<script>
  function getPeople() {
    return Object.values($roomState.users)
  }
</script>

{#each getPeople() as user (user.name)}
  {user.value}
{/each}

@brunnerh
Copy link
Member

brunnerh commented Mar 4, 2024

It works with $state because of fine-grained, deep reactivity.
E.g. the object with the value property can be extracted via $derived, modified and the change will update the UI since a signal on that object is fired, not the original state where the object is from (REPL).

And yes, indirection complicates this matter...

@Prinzhorn
Copy link
Contributor

Yes, but that logic can be in another function too.

I see, not sure if anyone would expect that to work (unless you change getPeople itself). At least in my mental model that doesn't make any sense why it should work, nothing referenced in the template changes. But I feel like Svelte 5 is questioning my mental model quite a bit.

@alex-way
Copy link
Author

alex-way commented Mar 4, 2024

Apologies if I oversimplified the original example, a slightly more complicated but hopefully more informational REPL can be found here of what I was actually doing: REPL

Effectively I'm attempting to use a derived store value. Again I could totally be wrong, but the example works with the Svelte 4 compiler but not Svelte 5 unfortunately

@dummdidumm
Copy link
Member

I think this was introduced through #10537 - strict equality checks now happen in runes mode instead of in immutable mode. I think we can add that strict equality only if we can statically determine that the state we're listening to is $state. In the end it might be easier to just always use mutable_source - but this probably has bad performance implications. I wonder if we need to change the logic around each blocks to not create an array of signal entries, if that's possible.

@trueadm trueadm self-assigned this Mar 4, 2024
@trueadm
Copy link
Contributor

trueadm commented Mar 4, 2024

@dummdidumm I'll take a look at this tomorrow and see what we can do :)

@brunnerh
Copy link
Member

brunnerh commented Mar 5, 2024

Additional PRs related to this:

(Fixing #each + $store scenarios using index or entire items as keys.)

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

Successfully merging a pull request may close this issue.

5 participants