State is undefined in cleanup functions when component is recreated with `{#key}`

[zeroberry]

Describe the bug

State becomes undefined in component cleanup functions (both onMount return functions and onDestroy callbacks) when the component is recreated using a {#key} block.

Reproduction

// App.svelte
<script>
  import { onMount } from 'svelte'
  import Component from './Component.svelte'

  let key = $state(0)

  onMount(() => {
    key = 1
  })
</script>

{#key key}
  <Component />
{/key}

// Component.svelte
<script>
  import { onMount } from 'svelte'

  let intervalId = $state()

  onMount(() => {
    intervalId = setInterval(() => {}, 1000)

    return () => {
      if (intervalId === undefined) throw new Error('intervalId is undefined')
          // ^? undefined
      clearInterval(intervalId)
    }
  })
</script>

https://svelte.dev/playground/6cece48d922a425d9be411b8711036aa?version=5.25.3

Logs

System Info

unnecessary

Severity

annoyance

[zeroberry]

It seems this isn’t limited to {#key}
The same issue can occur when a component is unmounted during the parent’s onMount.

// App.svelte
<script>
	import { onMount } from "svelte"	
	import Component from './Component.svelte';

	let show = $state(true);

	onMount(() => {
		show = false;
	})
</script>

{#if show}
	<Component />
{/if}

[Tichss]

Dont use $state on intervalId, just: let intervalId;

[zeroberry]

@Tichss Thanks! But this is just a simplified example for reproduction purposes. The issue is $state becoming undefined during cleanup function. Also, in legacy mode, let can still be a reactive state, so the solution you mentioned can't be applied in that case.
playground reproduction using legacy mode let

[brunnerh]

I usually keep the state entirely local if possible, i.e.

$effect(() => {
	const handle = setInterval(() => /* ... */, 1000)
	return () => clearInterval(handle);
});

There is no way for this variable to change.

[Tichss]

https://svelte.dev/playground/08fd26bd386542d0a11873f616c09e1e?version=5.25.3

then add await tick() to destroy function.

[zeroberry]

Thanks for the suggestions! I discovered this issue while debugging problems during Svelte 4 to 5 migration. While there are workarounds available, this appears to be an unexpected behavior change in Svelte 5. I believe it would be beneficial to document this difference.

Reference (Svelte 4): https://svelte.dev/playground/3cbf4f16aff34da1ace6622094f027ab?version=4.2.19

[gyzerok]

I believe this should have been handled in 5.23.0: #15469, but probably this specific scenario was missed somehow.

@trueadm am I correct here?

[trueadm]

It looks like this is working as intended. You're updating the key block in the mount phase, so it will use the previous value – which is undefined at this point. If I change it so you update the key block after the mount phase then you get the interval id.

[gyzerok]

My understanding was that the intended behavior is that you see the same value in teardown as you see in the body of onMount/$effect. So it's not necessarily previous one. This is how I've read Richs proposal at least

[trueadm]

@gyzerok That is how it is working, its providing the value it was when the effect was called – which is undefined. This is important because if we read the latest mutation from the effect then you'd have tearing. Like what if I had two effects?

onMount(() => {
  intervalId = setInterval(() => {}, 1000)
  console.log(intervalId);
  return () => {
    console.log(intervalId);
  }
});

onMount(() => {
  console.log(intervalId);
  return () => {
    console.log(intervalId);
  }
});

[gyzerok]

@trueadm personally I expect svelte here to behave like it inserts const value = value before teardown:

onMount(() => {
  intervalId = setInterval(() => {}, 1000)
  const intervalId = intervalId;
  console.log(intervalId);
  return () => {
    console.log(intervalId);
  }
});

onMount(() => {
  const intervalId = intervalId;
  console.log(intervalId);
  return () => {
    console.log(intervalId);
  }
});

So that if we do console.log(something) in both body of onMount and its teardown function it shows the same result. That was my expectation after that change in 5.23 at least. I feel like this is intuitive, but maybe I understood it wrongly.

[trueadm]

@gyzerok That's not how this fix is implemented though, it's implemented on using the previous value that was read. Not the previous value that was written to, if I make the change to like how you say a bunch of tests now start failing.

[gyzerok]

@trueadm was a bit challenging to find from which comment I've got the idea, but got it now (see in the answer to 3): #15469 (comment).

The guarantee is that the value will be the same in the teardown function as in the effect itself.

So either I misinterpret this sentence or it is closer to how I expect it to behave. And since you've said currently it's not working this way the question is if it should be adjusted to work this way or not? I'd advocate to adjust :)

[trueadm]

You're missing this statement in the same post:

If changesSometimes changes — in other words, that change is the reason for the effect to re-run — it will see the prior value.

The reason that is important is because both components are using onMount to do mutations. The parent component is doing this:

onMount(() => {
	key = 1;
})

onMount or $effect run in the child before the parent, so it will first run in the child component, where this happens:

intervalId = setInterval(() => {}, 1000)

Then because onMount or $effect will run in sync with other ones, then this occurs:

key = 1

That invalidates the update, so both changes are now going to be rolled back to their previous value during teardown, including:

intervalId = setInterval(() => {}, 1000)

State changes that happen as part of the same sync work all fall into the same bucket. That's just how signal based systems work – they use microtasks to break up these cycles.

If you break up the changes into different updates then things work as you'd expect:

onMount(() => {
  setTimeout(() => {
	key = 1;
  });
})

FWIW, this is a form of state tearing issue. We don't recommend making state changes in effects, as you can get tearing issues like this because of the fact that effects are meant to handle side-effects of external systems, not ones on the internal system. However you can easily work around it by doing queueMicrotask or setTimeout around the state change in your effect to avoid this class of problem.

[trueadm]

I found out there was a subtle bug that actually was the cause for this in the end, so I'll put up a PR to address this issue. Thanks for bearing with me.