`$derived.by` destructor support (Svelte 5)

[timephy]

Describe the problem

I use $derived or $derived.by to "couple" state widely in my app. For example like so:

    const baseObj = $state(...)
    const derivedObj = $derived.by(() => {
        // Not possible here, because it cannot reference itself
        // derivedObj.destroy()

        const _obj = new DerivedObj(baseObj)
        _obj.setup()
        return _obj
    })

The baseObj might not be a primitive type, but rather a class with "complex behaviour", for example in many cases a rxjs.Observable inside. When I create a new DerivedObj(baseObj) from that baseObj, then I might want to setup listeners/subscriptions on baseObj, which should be unsubscribed/destroyed on the old derivedObj, when it is recreated by $derived.

This raises the need for a destructor function.

Describe the proposed solution

$effect currently has something like "destuctor support": (see Preview Docs)

You can return a function from $effect, which will run immediately before the effect re-runs, and before it is destroyed (demo)."

Therefore it might make sense to implement it similarly for $derived.

There should be a functionality that does this (sentence from $effect docs reformulated):

You can specify a "destuctor" function, which will run immediately before the derived.by re-runs, and before it is destroyed.

I could imagine using it like so:

    const baseObj = $state(...)
    const derivedObj = $derived.by(
        // "constructor"
        () => {
            const _obj = new DerivedObj(call, modalVC)
            _obj.setup()
            return _obj
        },
        // "destructor"
        () => {
            derivedObj.destroy()
        }
    )

The destructor should be run immediately before $derived reruns and creates a new object from the source state. And before $derived is destroyed, so that the destructor is also called when a component is destroyed, otherwise we have to repeat the destructor inside onDestroy(() => derivedObj.destroy()), which would be repeating every time.

Importance

i cannot use svelte without it

[dummdidumm]

I think the better (and more broadly applicable) solution would be to provide the current value to the callback function.

$derived.by((curr) => {
  curr.destroy();
  return ...
})

[timephy]

What should in your opinion happen on the first initialization? Then curr == null?

[paoloricciuti]

What should in your opinion happen on the first initialization? Then curr == null?

Yeah either null or undefined

[timephy]

What should in your opinion happen on the first initialization? Then curr == null?

Yeah either null or undefined

If that is the solution, then most usages of this would look like this:

const foo = $derived.by((curr) => {
  if (curr) curr.destroy() // curr?.destroy()

  return ...
})

It would mean that you have to use if (foo) foo.destroy() or foo?.destroy(), every time.
Therefore destroy() not being called, or being called, only depending on if it is the first "iteration", or not. It think this logic should be abstracted away by giving a seperate function for destroying.

As far as I am aware $effect does not use a parameter for the returned destructor function either, and one is able to access all variables freely. I think this should apply to $derived.by's destructor as well.

const foo = $derived.by(
  () => new Foo(),
  () => foo.destroy() // or even just `foo.destroy`
)

[paoloricciuti]

The point is that having the current value can be useful for more things than just cleanup. So while it's annoying to have to check for the first instance it's not much more code than writing a new function...and since we've come this far away without even having the initial value I don't think cleaning up stuff in derived will be as common (especially because in theory they should be side effect free)

[timephy]

I can see some advantages to doing it your way too! This could also solve something that I have struggled with before:

I wanted to "propagate" state as if I was subscribing to an Observable, like so:

const x = $state(0)
const foo = new Foo()
$effect(() => {
    foo.update(x + 1)
}) // basically `x.subscribe((x) => foo.update(x + 1))`
// sadly not possible in `.svelte.ts` files

With this implementation, something like this would be possible:

const x = $state(0)
let foo = $derived.by((curr) => {
    // init if undefined
    if (!curr) {
        curr = new Foo()
    }
    
    // update
    curr.update(x + 1)
    return curr
})

I like that, because it allows me to do things that I used to do when using rxjs.Observables, but better.

(One more thing: if undefined | null is used for when the value is still uninitialized, then we can not differ between "uninitialized" (first run) or that the value actually IS undefined | null – might not be a problem, but worth thinking about)

[paoloricciuti]

Yeah good point on that...also @dummdidumm i have an implementation of this locally but there's a problem. We access props like this prop_name() and set the value like this prop_name(34). This doesn't create much of a problem per se but there's an optimization in b.thunk that eliminate the thunk if the argument is a call with 0 arguments (like the prop get)...however if derived now passes the value as the first argument something like this

<script>
    let { my_prop } = $props();
    const derived_prop = $derived(my_prop);
</script>

receive the first argument everytime and instead of reading sets the value. I've tried adding a force parameter to the b.thunk function to avoid removing the optimization all together but it's very brittle this way and if in the future we have a similar problem with other functions that could lead to a very bad experience for whoever has to work on that part of the codebase. Wdyt?

[Rich-Harris]

think the better (and more broadly applicable) solution would be to provide the current value to the callback function

I'm 👎 on this, it's an invitation to add complexity and it comes with all the aforementioned wrinkles. There are two reasons you might want the previous value:

1. So that you can do cleanup work (i.e. the case at hand) or other side-effects
2. So that the previous value can be an input to the current value

These are very different cases but they're both things that should generally be discouraged. In the first case, if you have stuff that needs to happen on cleanup, you can do this (i.e. stash the previous value yourself, rather than having it passed to you, which is the same except that a) Svelte is looking the other way rather than actively promoting it, and b) it doesn't take account of any future ambitions to add forking to the framework) but there's a bug — the cleanup doesn't happen when the component is destroyed, which might be acceptable in some scenarios but is absolutely not acceptable as a general pattern. If you have cleanup work to do, use an effect. It's fine. That's what they're for.

In the second case, while there might be some very rare cases where it's valid to treat the previous value, it's likely a sign that things need to be rethought a bit. What if you need the previous-but-one value? And so on. Providing it directly to the derivation might be convenient in a handful of cases but it makes it impossible to infer the correct type, and — again — it actively promotes something we should likely discourage.

It's possible that as the framework evolves I'll need to update this stance and we should consider passing in the previous value, but that's a bridge to cross then — for now it feels reckless.

[ryanatkn]

If you have cleanup work to do, use an effect. It's fine. That's what they're for.

Any thoughts on SSR? Related: #9278

[paoloricciuti]

Considering the amount of catches the implementation still has and that objectively even if a bit out of the ordinary (deriveds with side effects and assigning state in effects) a solution is possible in user land while also avoid constraining the framework I'm 100% on Rich side here.

[paoloricciuti]

If you have cleanup work to do, use an effect. It's fine. That's what they're for.

Any thoughts on SSR? Related: #9278

You can just initialize thing with new Thing right?

[ryanatkn]

You can just initialize thing with new Thing right?

How would you do cleanup during SSR? I can see using onDestroy, but it complicates things with duplicated cleanup, and on the client you'd need to guard against calling it both there and in the effect because calling it twice may cause problems depending on the implementation. You'd also need additional code to avoid churn on mount on the client, otherwise you'd create and immediately destroy and recreate the instance.

So if SSR was a concern I think you'd want to use the $derived.by version in combination with onDestroy like this. The main downside with my adjustment is that we're calling destroy in two places. The effect version doesn't seem compatible with these needs and SSR with the current behaviors.

[timephy]

These are very different cases but they're both things that should generally be discouraged. In the first case, if you have stuff that needs to happen on cleanup, you can do this (i.e. stash the previous value yourself, rather than having it passed to you

While I appreciate the want to keep such things away from the framework, sadly more complex things than primitive mappings are common in many projects... To illustrate how this issue would/could provide a simpler solution:

// A: currently possible
const x = $state(0)
const thing: Thing | null = null
const y: Thing = $derived.by(() => {
    thing?.destoy()
    thing = new Thing(x)
    return thing
})
onDestroy(() => {
    thing?.destroy()
})

// B: with solution in PR
const x = $state(0)
const y = $derived.by((curr) => {
    curr?.destroy()
    return new Thing(x)
})
onDestroy(() => {
    y.destroy()
})

// C: other possible solution
const x = $state(0)
const y = $derived.by(
    () => new Thing(x),
    (curr) => curr.destroy() // or `() => y.destroy()` or even `y.destoy`
)

I believe a need for this does occur, because it bridges a gap between derived-rune-land and imperative-land, and many would benefit from being able to do this more easily.

When looking at the code above I feel like A is the anti-pattern, not B or C.

the cleanup doesn't happen when the component is destroyed, which might be acceptable in some scenarios but is absolutely not acceptable as a general pattern.

The solution C would be more in line with how the $effect cleanup works and also keep the first argument as simple as it currently is.

And would also solve this bug/behaviour.

If you have cleanup work to do, use an effect. It's fine. That's what they're for.

This does not work in .svelte.ts files, which is a limitation that I ran into before, using $derived works and would create a unified solution for .svelte and .svelte.ts files together.