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

Binding to store values not working correctly in Svelte 5 #11626

Open
Ameobea opened this issue May 14, 2024 · 5 comments
Open

Binding to store values not working correctly in Svelte 5 #11626

Ameobea opened this issue May 14, 2024 · 5 comments

Comments

@Ameobea
Copy link

Ameobea commented May 14, 2024

Describe the bug

I'm seeing issues with binding to values within stores in Svelte 5. Simple code that I trivially ported from Svelte 4 experiences issues with the the store not updating through a bind.

I've attached two very small (~10 LOC) playground links that demo the issue. It's possible that perhaps I was mis-using stores or relying on non-guaranteed behavior in the past, but this functionality did indeed work in Svelte 4 and broke my app when I started porting over some components to use runes.

Here's the code for the broken playground inline for convenience:

App.svelte:

<script>
	import { writable } from 'svelte/store';
	import Checkbox from './Checkbox.svelte';
	
	let state = writable({ checked: true });
	$inspect({ App: state });
</script>

<Checkbox bind:state={$state} />

Checkbox.svelte:

<script>
	let { state = $bindable() } = $props();
	$inspect({ Checkbox: state });
</script>

<input type="checkbox" bind:checked={state.checked} />
{#if state.checked}
  Checked
{/if}

Expected Behavior:

The value in the store is updated when the checkbox is toggled. The checkbox will switch from checked to unchecked and the text "Selected" will appear/dissappear. The $inspect runes will print out the values in both the Checkbox and App components when it's toggled.

Actual Behavior:

The checkbox does toggle between checked and unchecked when clicked.

However:

  • The $inspect runes don't print anything except on the initial render
  • The "Selected" text doesn't appear/disappear to match the state of the checkbox

This leads me to believe that the store isn't getting updated, or if it is things aren't changing in a way that triggers reactive re-renders.

Reproduction

Working playground (Svlete 4)

Broken playground (Svlete 5)

Logs

No response

System Info

Svelte 4 playground version: 4.2.17

Svelte 5 playground version: Unknown; assuming trunk.  Using https://svelte-5-preview.vercel.app/#

Severity

blocking an upgrade

@longnguyen2004
Copy link

longnguyen2004 commented May 15, 2024

From my understanding, I'd say that you're indeed misusing stores, and this is how Svelte 5 fine-grained reactivity works. You're not changing the value of $state itself, but merely mutating the $state object, which doesn't cause an update from the store's point of view.

The reason that it works in Svelte 4, is not because of store updates, but because of the compiler invalidating the whole state prop when you mutate state.checked.

function input_change_handler() {
	state.checked = this.checked;
	$$invalidate(0, state);
}

This makes $: console.log({ Checkbox: state }) works as expected, but comes with a cost of invalidating everything that depends on state, even though the actual value doesn't change (i.e. $: console.log(state.completely_unrelated_property) will also re-run when state.checked is changed, see REPL)

In Svelte 5, the compiler no longer "guesses" what changed, instead moving that to run-time using signals. And since there are no signals on state.checked, nothing gets updated. FYI, converting your example to use $state works as expected.

@brunnerh
Copy link
Member

Well, stores are still a supported part of Svelte 5 and they just do not have fine-grained reactivity.
So a case could be made that this should be backward compatible.

A problem probably is that the Checkbox component does not know that the property gets its value from a store. On the App side, access to the store is wrapped in a get/set pair:

Checkbox(node, {
	get state() {
		return $state();
	},
	set state($$value) {
		$.store_set(state, $$value);
	}
});

The setter is currently not invoked. Maybe some metadata could be passed along to make the Checkbox aware that a store is being passed in and that the setter needs to be called, e.g. by proxying the prop. Might be a bit complicated, though.

@trueadm
Copy link
Contributor

trueadm commented May 15, 2024

This looks to be working as expected for runes mode. Svelte 4 doesn't have runes mode, so you're not comparing apples to apples.

I wonder if you remove the usage of runes, does this issue go away? Then it will force the components into legacy mode which eagerly overreads because of the lack of fine grain reactivity. Legacy mode is also more aligned to how Svelte 4 works, so if there's a difference then that's likely a bug we need to fix.

@brunnerh
Copy link
Member

brunnerh commented May 15, 2024

If the Checkbox component is in legacy mode, it works as expected.

It will cause the state setter to be called in the binding.

$.bind_checked(
  input,
  () => state().checked,
  ($$value) => state((state().checked = $$value, state()))
);

@dummdidumm
Copy link
Member

I'd say this works as expected in runes mode, though the behavior is probably a bit surprising at first. I'm wondering if there's a way to warn in this case (in dev mode).

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

5 participants