Skip to content

Setting a store to undefined and then trying to use it in a rune throws state_unsafe_mutation error #12558

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

Closed
ndom91 opened this issue Jul 23, 2024 · 7 comments · Fixed by #12562

Comments

@ndom91
Copy link

ndom91 commented Jul 23, 2024

Describe the bug

Previously, in Svelte 4, you used to be able to set a variable to undefined and later set it to a store and meanwhile use it in reactive statements. While not ideal, this used to work.

In svelte 5 if you do something similar it'll throw the state_unsafe_mutation error. See reproduction below. It's probably right to error in this case, but the error is very misleading we spent hours trying to track it down yesterday 😅

Maybe for some context, our before / after:

Before

const prs = $derived($hostedListingServiceStore?.prs);
const listedPr = $derived($prs?.find((pr) => pr.sourceBranch === branch.upstreamName));

After

const prStore = $derived($hostedListingServiceStore?.prs);
const prs = $derived(prStore ? $prStore : undefined);
const listedPr = $derived(prs?.find((pr) => pr.sourceBranch === branch.upstreamName));

Reproduction

https://svelte-5-preview.vercel.app/#H4sIAAAAAAAAE1WMzQqDMBCEXyUsHloQeven0GfosenBmrFdMJuQrIKI7148FOlxZr5vVhp4RKbqsZJ0HlTRLUYqSZe4hzxjVFBJOUyp35sm94mjXq1YHaHGd8sLdw0JpjWTOAwscPU-90HyDwge-mF5m9YUDolnuFNxuOfaSnM5rsXKWvybG5Xkg-OB4ajSNGF7bl-Xn5hLwAAAAA==

Logs

No response

System Info

Severity

annoyance

@paoloricciuti
Copy link
Member

There's something definitely wrong in your reproduction: you are accessing the store inside the derived which means you get back the value but then you are trying to access the value from $derived with $ like if it was a store...was the intended reproduction something like this

<script>
	let maybeStore = undefined;
	const maybeSomething = $derived($maybeStore);
</script>

{maybeSomething}

instead?

@paoloricciuti
Copy link
Member

Regardless i think this is probably something worth exploring...deriving a store works fine so it's probably a correct bug. Will try to investigate later

@dummdidumm
Copy link
Member

I know where the problem lies, PR soon

@paoloricciuti
Copy link
Member

paoloricciuti commented Jul 23, 2024

I know where the problem lies, PR soon

Oh was about to comment...just to confirm...i fixed it by changing this

export function store_get(store, store_name, stores) {
	const entry = (stores[store_name] ??= {
		store: null,
		source: mutable_source(undefined),
		unsubscribe: noop
	});

	if (entry.store !== store) {
		entry.unsubscribe();
		entry.store = store ?? null;

		if (store == null) {
-			set(entry.source, undefined);
+			entry.source.v = undefined;
			entry.unsubscribe = noop;
		} else {
			var is_synchronous_callback = true;

			entry.unsubscribe = subscribe_to_store(store, (v) => {
				if (is_synchronous_callback) {
					// If the first updates to the store value (possibly multiple of them) are synchronously
					// inside a derived, we will hit the `state_unsafe_mutation` error if we `set` the value
					entry.source.v = v;
				} else {
					set(entry.source, v);
				}
			});

			is_synchronous_callback = false;
		}
	}

	return get(entry.source);
}

but you still need to wrap the actual store definition in $state or else the derived wont rerun. Did i miss something?

@dummdidumm
Copy link
Member

dummdidumm commented Jul 23, 2024

No that's exactly what I did as well 👍 And yes, the example needs $state or it won't rerun, and the additional $ in front of the derived value is wrong.

dummdidumm added a commit that referenced this issue Jul 23, 2024
We need to extend the "don't use `set` on first run" logic to the falsy branch aswell

Fixes #12558
@paoloricciuti
Copy link
Member

paoloricciuti commented Jul 23, 2024

No that's exactly what I did as well 👍 And yes, the example needs $state or it won't rerun, and the additional $ in front of the derived value is wrong.

waves-brain-gif-by-kidmograph

I will tell my grandkid i had the same fix as dummdidumm 😜

@ndom91
Copy link
Author

ndom91 commented Jul 23, 2024

Awesome thanks a lot for the quick turn around! 😊

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.

3 participants