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

$inspect() of shared external state fires unexpectedly in conditionally nested component #14891

Open
mjadobson opened this issue Jan 4, 2025 · 5 comments

Comments

@mjadobson
Copy link

Describe the bug

This was an unexpected behaviour I noticed:

Given an external state, where a parent component conditionally renders a child component based on it. If the child component references it in an $inspect(), the $inspect() will fire even though the component should have been destroyed.

This can lead to errors where you expect the external state to be a certain shape in the child component.

This doesn't happen with render or user effects.

Reproduction

https://svelte.dev/playground/d5df84a246d742308383c661d658bf81?version=5.16.1

Click Init Items, then Reset Items to show the behaviour.

Logs

No response

System Info

System:
  OS: macOS 15.0.1
  CPU: (8) x64 Intel(R) Core(TM) i5-1038NG7 CPU @ 2.00GHz
  Memory: 715.43 MB / 16.00 GB
  Shell: 5.9 - /bin/zsh
Binaries:
  Node: 20.12.1 - /usr/local/bin/node
  npm: 10.5.0 - /usr/local/bin/npm
Browsers:
  Chrome: 131.0.6778.205
  Safari: 18.0.1
npmPackages:
  svelte: ^5.16.1 => 5.16.1

Severity

annoyance

@Ocean-OS
Copy link
Contributor

Ocean-OS commented Jan 5, 2025

I have a feeling this may be an effect scheduling issue, since both render effects (the {#if} block, for example) and $inspect effects are synchronous, so there might be some sort of weird race condition. You can see what I mean here, where the code works because I set a timeout before the state's value is undefined.

@Leonidaz
Copy link

Leonidaz commented Jan 5, 2025

Similar to #14878

This is expected behavior. The component is still loaded when the signal is changed and the subscribers in the component are executed.

You either need to unload the component first, e.g. use another signal for the {#if} and use tick() to change the signal (allow component to unload), like so

<!-- App.svelte -->
<svelte:options runes />
<script>
	import Component from './Component.svelte';
	import { items } from './state.svelte.js';
	import { tick, untrack } from 'svelte';

	let shouldLoad = $state(false);

	function initItems() {
		items.value = [1,2,3];
		shouldLoad = true;
	}
	function resetItems() {
		shouldLoad = false;
		tick().then(() => items.value = undefined);
	}
	
</script>
<button onclick={initItems}>Init Items</button>
<button onclick={resetItems}>Reset Items</button>
{#if shouldLoad}
	<Component />
{/if}

Or, just account for the fact that your reactive data's shape may change and use optional chaining: items.values?.join(',')

<!-- Component.svelte -->
<script>
	import { items } from './state.svelte.js';

	let bar = $derived('bar: ' + items.value?.join(','));

	let baz = $state('baz: ' + items.value?.join(','));
	$effect(() => baz = 'baz: ' + items.value?.join(','));

	// TypeError: Cannot read properties of undefined (reading 'join')
	$inspect(items.value?.join());
	
	// TypeError: Cannot read properties of undefined (reading 'join')
	$inspect(bar);

	// ok
	$effect(() => console.log(bar));

	// ok
	$inspect(baz);
</script>
{bar}
{baz}

@mjadobson
Copy link
Author

This is expected behavior. The component is still loaded when the signal is changed and the subscribers in the component are executed.

Yeah, I thought this was the case initially too, but it seems to only be $inspect() with this behaviour. Other signal subscribers like $effect(), $effect.pre() and render effects don't seem to do this. $derived() can percolate the issue when used with $inspect(), but otherwise doesn't demonstrate it either.

@Leonidaz
Copy link

Leonidaz commented Jan 5, 2025

This is expected behavior. The component is still loaded when the signal is changed and the subscribers in the component are executed.

Yeah, I thought this was the case initially too, but it seems to only be $inspect() with this behaviour. Other signal subscribers like $effect(), $effect.pre() and render effects don't seem to do this. $derived() can percolate the issue when used with $inspect(), but otherwise doesn't demonstrate it either.

Yeah, so all effects are run on a microtask queue to batch updates. So when the signal changes, the component is unloaded and this is done first since it's in the parent component, higher up the component tree, which also removes all effects inside the component. However, $inspect runs immediately for every signal change because that's the whole purpose behind it to see every single signal change. If it ran on micro queue then it would not be able to see certain changes. If the component was not unloaded, you'd run into the same errors as the effects would fire and reference a non-existent property.

specifically about each instance of $inspect in your example (inspect registers a subscribe callback function that fires when the referenced signal changes)

  • this line $inspect(items.value.join()); fails because it's called as soon as items.value change.

  • line $inspect(bar); fails because the derived was marked as a dirty (changed) and reading the derived recalculates the derived by rerunning its callback, so it fails for the same reason because items.value is undefined.

    (incidentally, deriveds are only executed if read (unlike effect) and only marked as dirty (but not re-executed yet) if its signal dependencies change. If no change, its cached value is returned and the derived expression / callback is not re-executed.)

  • line $inspect(baz); is not fired because baz has not changed since baz is only updated in the effect, $effect(() => baz = 'baz: ' + items.value.join(','));, which is removed with the component. The baz creation - let baz = $state('baz: ' + items.value.join(',')); - only initializes the state but items.value is not a dependency and baz is not recalculated here ever again.

Lastly, even in the unloaded component you could still run into trouble if you reference the items.value.join(',') in the effect's return callback (runs before next effect / effect is destroyed)

Example with effect returning a ondestroy callback

So, I think, it's always a good practice to either not change the shape of signals or account for the changes in the code, e.g. via optional chaining.

@mjadobson
Copy link
Author

Interesting, thanks for the in-depth explanation.

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

3 participants