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: fromStore breaks reactivity when used with @const #13002

Closed
kosmotema opened this issue Aug 23, 2024 · 7 comments · Fixed by #13087
Closed

Svelte 5: fromStore breaks reactivity when used with @const #13002

kosmotema opened this issue Aug 23, 2024 · 7 comments · Fixed by #13087
Assignees
Milestone

Comments

@kosmotema
Copy link

Describe the bug

When using the value obtained from fromStore in the @const tag, reactivity starts to work strangely:

  • the first update of the store value updates only the block with @const, but not other blocks of the component
  • subsequent updates, on the contrary, do not update the @const block, but update other blocks

This behavior is only observed when fromStore is used. This is not the case when using reactive syntax (like $store, inclu) or manual subscription using runes (like $state + $effect)

Reproduction

https://svelte-5-preview.vercel.app/#H4sIAAAAAAAACrVUTW-cMBT8KxaNFEijpe1xy66SS6UeemmO3R5Y80itgo2MTVtZ_Pf6gzWY0N0QpQeEsOcNbzzjp6KSVNBG228qonkN0Ta6b5roNhJ_GvPRdlAJ0N8tkxyblazFnDRif6AHQeqGcYE-U8yhBipQyVmNrjepX9k4guuPE_hXyLEgHTwIxsGXBKtLZV9yKvPqQR5dA4RRX_t0a4ngk0aH__QrE3iWjgJpNkpL9UL2g7t3KCHYWmgz2B-7SPf6XGtWkJJAEW0Fl9Dfehtag3mGEUgzyAqmfij0ixORHytA_aDVlaeW1B2KeeC3xWNGW4HsHtr52vhdMj2Oc716TSuDo2x37jT-3akHtwFw4zBTpzXYaenyShotnj622GTur3pDSmTFmFp156oLJrX-Qtdbng2WnJsI3KAPFpcVpLMyDmJ0Mya0JQUkW6SCqt6UaSo1sDqG1FGolJS9DdqJckLIpDjPuNTdifmcXT7Uq-265EB4eY5SCB1_RnFF8M-dihO02yNlGF2hbIpcQBx3dqNDb9F7Y1Hf78mpxSx1LGfzF1zH_yvqUmKuHNtCVMKZMYmLK1mZkxnbJCszuqCh56Tj6QBbfaKPIF7rPlcw3mYtxcRFsw_XORlAV1CWgEXsAjbQOAFHH6_4RNMlycsGwZKrC-N-PglWOrvEOB8F4Qi4aO73_i_NGI9M6AcAAA==

Logs

No response

System Info

Severity

annoyance

@trueadm
Copy link
Contributor

trueadm commented Aug 24, 2024

This is because we're creating an effect inside a derived, which means that when the derived runs again, the inner effect is destroyed and the reactivity is lost. If we make $effect.tracking return false for inside a derived, then this also fixes the issue.

@stuartbritt
Copy link

I'm seeing really weird reactivity issues when using fromStore to runify the page store.

<script>
 function get_form(form_name: string) {
	const pagestore = fromStore(page);
	if (pagestore.current.form?.[form_name]) {
		return pagestore.current.form[form_name];
	} else if (pagestore.current.data?.[form_name]) {
		return pagestore.current.data[form_name];
	} else throw new Error('Unknown form');
}

	const form2 = $derived(get_form('form2'));
	const form1 = $derived(get_form('form1'));
</script>
<!-- Two forms here, using use:enhance and form actions-->
<pre>{JSON.stringify(form1.field_data.name, undefined, 2)}</pre> <!-- Not reliably reactive-->
<pre>{JSON.stringify(form2.field_data.name, undefined, 2)}</pre> <!-- Not reliably reactive-->
<Monitor {form1} {form2} /> <!-- Also displays a <pre> and reactivity works fine here-->

If I access form1 or form2 directly in the same page, the reactivity is inconsistently broken (it is really, really weird and I can't work out the pattern).
If I pass form1 or form2 to a component on the page, then it all works fine.
I tried using $derived and getters inside the function to avoid needing $derived in the main code and that had the same issue.

Could it be related?

@7nik
Copy link

7nik commented Aug 25, 2024

 function get_form(form_name: string) {
	const pagestore = fromStore(page);
	if (pagestore.current.form?.[form_name]) {
		return pagestore.current.form[form_name];
	} else if (pagestore.current.data?.[form_name]) {
		return pagestore.current.data[form_name];
	} else throw new Error('Unknown form');
}

	const form2 = $derived(get_form('form2'));
	const form1 = $derived(get_form('form1'));

It doesn't look like good code — it re-creates the state and re-subscribes to the store at each change.

It should be like this

 function get_form(form_name: string) {
	const pagestore = fromStore(page);
	if (pagestore.current.form?.[form_name]) {
		return () => pagestore.current.form[form_name];
	} else if (pagestore.current.data?.[form_name]) {
		return () => pagestore.current.data[form_name];
	} else throw new Error('Unknown form');
}

	const form2 = $derived.by(get_form('form2'));
	const form1 = $derived.by(get_form('form1'));

@trueadm trueadm added this to the 5.0 milestone Aug 30, 2024
@trueadm
Copy link
Contributor

trueadm commented Aug 30, 2024

I think we need to investigate this more before 5.0, something doesn't seem right here.

@stuartbritt
Copy link

stuartbritt commented Aug 30, 2024

Here's a simple reproduction of fromStore() behaving strangely. I'm not sure whether it's exactly the same issue as the above, but seems related.
REPL

(edited as posted wrong REPL!)

@pauldemarco
Copy link

pauldemarco commented Aug 30, 2024

I don't want to hijack this thread with a potentially different issue, but I am seeing fromStore lacking deep reactivity.

I am trying to convert the form store from superforms into a rune so I can pass it to a snippet and bind it to an input field:

const {form} = superform(...);
const formRune = fromStore(form);

{@render children({formRune})}
{#snippet children({formRune})}
<input bind:value={formRune.current.firstName} />
{/snippet}

I am getting binding_property_non_reactive errors.

@trueadm
Copy link
Contributor

trueadm commented Aug 30, 2024

Here's a simple reproduction of fromStore() behaving strangely. I'm not sure whether it's exactly the same issue as the above, but seems related. REPL

(edited as posted wrong REPL!)

I have a fix for this here #13087. It doesn't resolve the original issue though, so that might be a separate issue.

@trueadm trueadm self-assigned this Sep 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants