feat(computed): add cleanup method

[basvanmeurs]

See #2261 (comment)

The cleanup method will remove all dep references to and from dependency sets. This allows the computed to be GC collected (if no other references remain).

Another reason to call this cleanup is to reduce the number of dependencies. Triggering a ref may become slow when there are many stale computeds that depend on it - as they will all have to be triggered. When calling cleanup, you can reduce the amount of stale computeds.

Notice that the computed will remain active and fully functional after cleanup. By simply setting the dirty flag upon cleanup, when getting the computed value later, all dependencies will simply be recovered and the correct value will be returned!

[basvanmeurs]

One question @yyx990803
In the final commit, I clear the cached value upon cleanup.

Personally, I think that it should also be cleared immediately after setting the dirty flag in the scheduler. It will never be used anyway and will use memory until the computed is read again.

[yyx990803]

why is it necessary to set dirty here?

[basvanmeurs]

By setting the dirty flag upon cleanup, the computed will stay fully functional. When getting the computed value later, all dependencies will simply be recovered and the correct value will be returned. This way, it is possible to temporarily cleanup deps which in some cases can become a performance bottleneck.

[antfu]

This is indeed a good addition. However, I am a bit concern about the constancy of the APIs we are going to have.

Currently & On Going

To clean up effects:

Effect

import { stop, effect } from '@vue/reactivity'

const runner = effect(/* ... */)

stop(runner)

Watch / Watch Effect

import { watch } from 'vue'

const stopHandler = watch(/* ... */)

stopHandler()

Scope (PR #2195)

import { effectScope, stop } from '@vue/reactivity'

const scope = effectScope(/* ... */)

stop(scope)

Computed (this PR)

import { computed } from 'vue'

const foo = computed(/* ... */)

foo.cleanup()

Proposal

To have a general interface of disposable effect and let the return value of effect scope watch computed extends it

export interface Disposable {
  stop: () => void
}

const runner = effect(/* ... */)
runner.stop()

const scope = effectScope(/* ... */)
scope.stop()

const foo = computed(/* ... */)
foo.stop()

// as it's a breaking change, not sure if it's worth to do it
const watcher = watch(/* ... */)
watcher.stop()

Wondering what you think?

[basvanmeurs]

@antfu
This occurred to me as well, but..

computed.cleanup is different from watch stopHandles, and even effectScope.stop.

When running a stop on a watcher, it makes the effect inactive: it sets effect.active to false. It's really destroyed and can no longer be used.

In case of computed.cleanup it merely dereferences it from all dependencies. It does not make the effect inactive, and it could be re-used without problems.

Maybe I should provide some background on why this is important. I've ran into this situation in a complex scroller with 20'000 rows that all had computeds depending on one base computed. As more and more rows were scrolled into view, and more 'row computeds' were invoked, scrolling performance degraded. Why? The base computed, upon any trigger, had to trigger all those thousands of computeds that belonged to rows that were no longer 'within view'. Even though they were marked dirty a long time ago, the iteration process alone proved really wasteful. On top of that, as computeds cache their values, it held on to vast amounts of memory unnecessarily. Even when removing those rows later on, the base computed held the references causing a memory leak.

I think it's best to keep the method called cleanup. Another option would be to rename it to dereference or deref, as it is a closer description of what it does. But I don't think it's a good idea to name it 'stop' because it implies that it is no longer usable. As for effectScope, I think 'stop' is a good description. Personally I would have gone for destroy though, as the effectScope is beyond repear after invoking it, though all references are cleaned up.

[antfu]

I misunderstood the original motivation, thanks for the detailed explanation. The naming is good to me then 👍

[jods4]

I was intrigued when I saw this added to the 3.1 plan.

I think that a stop concept is needed (either in addition or in replacement of cleanup).

The issue #2261 notes that a computed having a long-lived dependency will leak itself and all its dependencies.

Looking at WPF for prior art, the "best" (but not really, read on) solution is to use WeakRef for dependency tracking. A weak ref means that a long-lived dependency would not keep a computed alive if nothing can read from that computed anymore.

Problem is that weak references are a recent JS addition. They won't work in pre-2020 browsers, won't work in neither Safari nor Opera, and it's not something you can polyfill. So I suppose they're out of question here.

This means that every effect depending on long-lived dependencies must be carefully tracked and stopped -- otherwise one leaks memory.

There are high-level tools for that in Vue: it's done automatically in components, which are a special case of (upcoming) effectScope.

It seems natural to me that there should be a low-level stop concept as well, for the simple case where a user wants to stop a single computed that they created.

I don't know if a cleanup is needed. I'd tend to think you could destroy (stop) and re-create computeds just as well but I didn't fully grasp the background given by @basvanmeurs. So why not.

When it comes to bikeshedding the name, I find cleanup not a great name. It conveys an after-stop / dispose idea that's more about the internals than the use-cases. Other ideas: pause or suspend? In contrast from stop it conveys that the computed would not be active anymore but ccould be started again.

[Valter4o]

Hey

I was intrigued as well by the changes to the effectApi and cleanup() addition. I haven't gotten to the point where I need them in a project ( surely I will do soon ) but I really think that the differences in the API's would be a struggle for the every devs that want to use them

I get that there is a difference on low level or how they behave but most of the people who will use it won't really care about that. The only thing they would want is just to stop something from being reactive

If I know that when I define const myComputedProp = computed(()=>{}) and then can stop it with just myComputedProp.stop(). Tomorrow when I have const myEffect=effect(()={}) I will probably first try myEffect.stop()

And as I think more about it maybe stop is not the best name, maybe as mentioned above dereference or deref, would be a better fit, or dereact ( as of de-React * badum-ts * ), jokes aside I think standarized API would be easier to use and more intuitive

Cheers

[basvanmeurs]

Problem is that weak references are a recent JS addition. They won't work in pre-2020 browsers, won't work in neither Safari nor Opera, and it's not something you can polyfill. So I suppose they're out of question here.

Safari now supports this as well. Now that IE11 support has been dropped, that only leaves Opera. WeakRef provides a native solution for cleaning up. Notice that it also provides a solution for the current problem where async created computes will leak even when they are created from within a component.

I actually tested a basic implementation by simply replacing the Dep set (Set<ReactiveEffect>) by Set<WeakRef<ReactiveEffect>> (https://github.com/Planning-nl/vue-next/commit/701e4e06ff4d67d5ab941da582b216c77ef307d0). I found little to none performance effect.

When using WeakRefs, you really need to reference a watcher, because if you don't, it will not be strongly referenced any more and removed automatically:

this.myWatcher = watch(a, () => {})

I tried this in my own project, but it doesn't feel natural. I think mainly because in a component setup this is not required (as they are referenced in instance.effects). Perhaps effectScope provides a more intuitive solution. I reviewed it and it looks good.

[basvanmeurs]

Let's close this PR because of the following reasons:

• computeds can be stopped by wrapping it in an effectScope (feat(reactivity): new effectScope API #2195), and stopping that
• for convenience and to avoid having to create effectScopes, we could add a stop method to the computed class which simply invokes this.effect.stop(), but that might be better suited for a separate PR (or @yyx990803 could just commit it to 3.2 directly)
• on second thought, cleaning up references (which is an edge case) can be done manually by re-creating the same computed and exposing it wrapped in a ref