Bad/out-of-date state in effect teardowns

[rChaoz]

Describe the bug

In short:

let thing = $state(true)

// no longer true!!
thing = false

// later
if (thing) doSomething() // this executes, but it shouldn't

Initially my code had let thing = true. I want to expose the state for use in UI in a template (so I change it to use $state), but this breaks the existing code, as for whatever reason getting the variable returns true even after it's set to false.

Reproduction

https://svelte.dev/playground/f81b82a8cefd45da95d8301e82b0d5d4?version=5.33.4

Update: Better repro - https://svelte.dev/playground/a8c378e462544c9baf1b5f22e0531185?version=5.33.4

Severity

important

Workaround

Currently I use:

let thing = false
let thing2 = $state(false)

function setThing(value) {
  thing = value
  thing2 = value
}

// code keeps using non-reactive state, so it works
if (thing) doSomething()

// in template, where reactivity is needed, use thing2

This is much less than ideal

[dummdidumm]

@Rich-Harris correct me if I'm wrong but this is expected behavior since #15469 - in the general case it's confusing to get the new value in the teardown.

[rChaoz]

That sounds... really bad. Isn't the whole point of Svelte that a reactive variable is just a variable?

Unlike other frameworks you may have encountered, there is no API for interacting with state — count is just a number, rather than an object or a function, and you can update it like you would update any other variable.

[...]

State in Svelte is no different — when you reference something declared with the $state rune, you’re accessing its current value.

So this is just... not true anymore? I'm developing a library function and using the getter pattern described in the docs, which massively advertises states as regular variables:

If add wanted to have access to the current values of a and b, and to return the current total value, you would need to use functions instead

That's exactly what I'm doing, and instead of getting the current value, I'm getting... I don't even know what I'm getting, because I don't know if the library user calls my function in a teardown or not. Am I supposed to just put into the documentation of my function "hey, don't call this function from teardowns"?

As for the point of the PR you mentioned, basically, because it was too difficult for users coming from React to change this code...

element.addEventListener(...)
return () => element.removeEventListener(...)

...into this...

const e = element
e.addEventListener(...)
return e.removeEventListener(...)

...it is now impossible to get the true value of a state (which makes writing library code that works correctly everywhere, including teardowns, impossible). Not to mention, this breaking change was pushed in a non-major version, and without any documentation. I'm terribly sorry for the rant and my emotional reply, but this seems like a massive oversight.

[rChaoz]

(sorry again, just want to get the last bit of anger out)

To quote the PR:

$effect(() => {
  const prev = value;
  return () => {
    // could be `true` but is usually `false`, depending on
    // whether the teardown is running because the effect
    // is about to re-run (because `value` changed) or
    // because it's being destroyed (because the
    // component unmounted, for example)
    console.log(prev === value); 
  };
});

Yeah, and this code is also confusing as hell for people coming from React:

const prev = value
doSomething()
// oh not, doSomething might've changed the value, this might be false
console.log(prev === value)

While at it, let's also make it so that the condition above is always true, like it would be in React.

sorry again

[rChaoz]

Back to sanity. Here's my (simplified) use-case in short:

class Draggable {
  #dragging = $state(false)

  onDragStart() {
    this.#dragging = true
  }

  onDragEnd() {
    this.#stopDrag()
  }

  #stopDrag() {
    if (this.#dragging) dragFinishCallback()
    this.#dragging = false
  }

  mount(elem) {
    this.elem = elem
    elem.addListeners(...)
  }

  unmount() {
    this.elem.removeListeners(...)

    // don't get stuck in dragging state if element is removed while dragged
    this.#stopDrag()
  }
}

Used something like:

const d = new Draggable()
onMount(() => {
  d.mount(elem)
  return () => d.unmount()
}

And, right now, the dragFinishCallback runs again if the first run causes the element to be unmounted, which is neither intended nor expected.

This is a quite simplified and maybe even a bit of an edge case, but the point stands: with this behaviour, it is impossible to write a standalone reactive class, as you can't guarantee methods are not called from a teardown (one way or another, sometimes through a chain of multiple calls).

[paoloricciuti]

The point of that or is that in situation like this

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

If inside component you have this

$effect(()=>{
    console.log(obj.mystuff);
    return ()=>{
        console.log(obj.mystuff);
    }
});

You generally think that since you guarded the component with an if you are safe...but since that cleanup runs because obj is undefined it actually errors out.

This has some downside in cases like yours where you want to get the very latest value. This is definitely confusing and I wonder if we can find a solution, however: what are you trying to do that you need the value to be the updated one in the teardown?

[rChaoz]

@paoloricciuti please see the reply above, posted just seconds before your question

Also, for

$effect(()=>{
    console.log(obj.mystuff);
    return ()=>{
        console.log(obj.mystuff);
    }
})

why not write

$effect(()=>{
    const s = obj.mystuff;
    console.log(s);
    return ()=>{
        console.log(s);
    }
});

and how is it different from

console.log(obj.mystuff);
setTimeout(() => {
  console.log(obj.mystuff);
});

...and why does Svelte need to handle it specifically/differently?

[paoloricciuti]

@paoloricciuti please see the reply above, posted just seconds before your question

Yeah I was looking at it

[rChaoz]

Here's a much better explanation of the issue, and how easily external/library code (a simple sum += value in a .svelte.js file) can break down completely.

The idea is that anytime you have something like:

$effect(() => {
    someLibraryCode()
    return () => someMoreLibraryCode()
})

As long as the code handles reactive state in an async manner (animations, network requests, delays, debouncing...), there is a very high chance it will break, and in a very difficult to track down way. There is nothing the library author can do other than place a Do not call this function in effect teardowns warning in the JSDoc. And even then, the issue can appear if the function is called in another function, and the disclaimer isn't copied over.