fix: use FinalizationRegistry to cleanup unused computed

packages/reactivity/__tests__/computed.spec.ts

@@ -11,6 +11,7 @@ import {
   ITERATE_KEY,
   TriggerOpTypes
 } from '../src'
+import { depsMap } from '../src/effect'
 
 describe('reactivity/computed', () => {
   it('should return updated value', () => {
@@ -400,10 +401,11 @@ describe('reactivity/computed', () => {
     a.value++
     e.value
 
-    expect(e.effect.deps.length).toBe(3)
-    expect(e.effect.deps.indexOf((b as any).dep)).toBe(0)
-    expect(e.effect.deps.indexOf((d as any).dep)).toBe(1)
-    expect(e.effect.deps.indexOf((c as any).dep)).toBe(2)
+    const deps = depsMap.get(e.effect._trackToken)
+    expect(deps!.length).toBe(3)
+    expect(deps!.indexOf((b as any).dep)).toBe(0)
+    expect(deps!.indexOf((d as any).dep)).toBe(1)
+    expect(deps!.indexOf((c as any).dep)).toBe(2)
     expect(cSpy).toHaveBeenCalledTimes(2)
 
     a.value++

packages/reactivity/__tests__/gc.spec.ts

@@ -2,9 +2,11 @@ import {
   ComputedRef,
   computed,
   effect,
+  reactive,
   shallowRef as ref,
-  stop
+  toRaw
 } from '../src/index'
+import { getDepFromReactive } from '../src/reactiveEffect'
 
 describe('reactivity/gc', () => {
   const gc = () => {
@@ -35,7 +37,21 @@ describe('reactivity/gc', () => {
     expect(srcRef.deref()).toBeUndefined()
   })
 
-  it('should not release effect', async () => {
+  it('should release reactive property dep', async () => {
+    const src = reactive({ foo: 1 })
+
+    let c: ComputedRef | undefined = computed(() => src.foo)
+
+    c.value
+    expect(getDepFromReactive(toRaw(src), 'foo')).not.toBeUndefined()
+
+    c = undefined
+    await gc()
+    await gc()
+    expect(getDepFromReactive(toRaw(src), 'foo')).toBeUndefined()
+  })
+
+  it('should not release effect for ref', async () => {
     const spy = vi.fn()
     const src = ref(0)
 
@@ -50,4 +66,20 @@ describe('reactivity/gc', () => {
     src.value++
     expect(spy).toHaveBeenCalledTimes(2)
   })
+
+  it('should not release effect for reactive', async () => {
+    const spy = vi.fn()
+    const src = reactive({ foo: 1 })
+
+    effect(() => {
+      spy()
+      src.foo
+    })
+
+    expect(spy).toHaveBeenCalledTimes(1)
+
+    await gc()
+    src.foo++
+    expect(spy).toHaveBeenCalledTimes(2)
+  })
 })

packages/reactivity/__tests__/readonly.spec.ts

@@ -10,6 +10,7 @@ import {
   isProxy,
   computed
 } from '../src'
+import { depsMap } from '../src/effect'
 
 /**
  * @see https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-4.html
@@ -393,7 +394,8 @@ describe('reactivity/readonly', () => {
     const eff = effect(() => {
       roArr.includes(2)
     })
-    expect(eff.effect.deps.length).toBe(0)
+    const deps = depsMap.get(eff.effect._trackToken)
+    expect(deps).toBeUndefined()
   })
 
   test('readonly should track and trigger if wrapping reactive original (collection)', () => {

packages/reactivity/src/effect.ts

@@ -21,7 +21,35 @@ export type DebuggerEventExtraInfo = {
 
 export let activeEffect: ReactiveEffect | undefined
 
-class FakeWeakRef<T> {
+let _FinalizationRegistry = getGlobalThis()
+  .FinalizationRegistry as typeof FinalizationRegistry
+
+if (!_FinalizationRegistry) {
+  _FinalizationRegistry = class FakeFinalizationRegistry {
+    register() {}
+    unregister() {}
+  } as any
+  if (__DEV__) {
+    console.warn(`FinalizationRegistry is not available in this environment.`)
+  }
+}
+
+const registry = new _FinalizationRegistry<WeakRef<ReactiveEffect>>(
+  trackToken => {
+    const deps = depsMap.get(trackToken)
+    if (deps) {
+      for (const dep of deps) {
+        dep.delete(trackToken)
+        if (dep.size === 0) {
+          dep.cleanup()
+        }
+      }
+      deps.length = 0
+    }
+  }
+)
+
+class StrongRef<T> {
   constructor(public target: T) {}
   deref() {
     return this.target
@@ -31,15 +59,16 @@ class FakeWeakRef<T> {
 let _WeakRef = getGlobalThis().WeakRef as typeof WeakRef
 
 if (!_WeakRef) {
-  _WeakRef = FakeWeakRef as any
+  _WeakRef = StrongRef as any
   if (__DEV__) {
-    console.warn(`WeakRef is not available domain-wide and must be stubbed!`)
+    console.warn(`WeakRef is not available in this environment.`)
   }
 }
 
+export const depsMap = new WeakMap<WeakRef<ReactiveEffect>, Dep[]>()
+
 export class ReactiveEffect<T = any> {
   active = true
-  deps: Dep[] = []
 
   /**
    * Can be attached after creation
@@ -76,18 +105,23 @@ export class ReactiveEffect<T = any> {
   public get dirty() {
     if (this._dirtyLevel === DirtyLevels.ComputedValueMaybeDirty) {
       this._dirtyLevel = DirtyLevels.NotDirty
-      this._queryings++
-      pauseTracking()
-      for (const dep of this.deps) {
-        if (dep.computed) {
-          triggerComputed(dep.computed)
-          if (this._dirtyLevel >= DirtyLevels.ComputedValueDirty) {
-            break
+      if (this._trackToken) {
+        const deps = depsMap.get(this._trackToken)
+        if (deps) {
+          this._queryings++
+          pauseTracking()
+          for (const dep of deps) {
+            if (dep.computed) {
+              triggerComputed(dep.computed)
+              if (this._dirtyLevel >= DirtyLevels.ComputedValueDirty) {
+                break
+              }
+            }
           }
+          resetTracking()
+          this._queryings--
         }
       }
-      resetTracking()
-      this._queryings--
     }
     return this._dirtyLevel >= DirtyLevels.ComputedValueDirty
   }
@@ -103,9 +137,10 @@ export class ReactiveEffect<T = any> {
     }
     if (!this._trackToken) {
       if (this.scheduler) {
-        this._trackToken = new FakeWeakRef(this) as any
+        this._trackToken = new StrongRef(this) as any
       } else {
         this._trackToken = new _WeakRef(this)
+        registry.register(this, this._trackToken, this)
       }
     }
     let lastShouldTrack = shouldTrack
@@ -114,7 +149,7 @@ export class ReactiveEffect<T = any> {
       shouldTrack = true
       activeEffect = this
       this._runnings++
-      cleanupEffect(this)
+      preCleanupEffect(this)
       return this.fn()
     } finally {
       postCleanupEffect(this)
@@ -126,7 +161,7 @@ export class ReactiveEffect<T = any> {
 
   stop() {
     if (this.active) {
-      cleanupEffect(this)
+      preCleanupEffect(this)
       postCleanupEffect(this)
       this.onStop?.()
       this.active = false
@@ -138,17 +173,22 @@ function triggerComputed(computed: ComputedRefImpl<any>) {
   return computed.value
 }
 
-function cleanupEffect(effect: ReactiveEffect) {
+function preCleanupEffect(effect: ReactiveEffect) {
   effect._trackId++
   effect._depsLength = 0
 }
 
 function postCleanupEffect(effect: ReactiveEffect) {
-  if (effect.deps.length > effect._depsLength) {
-    for (let i = effect._depsLength; i < effect.deps.length; i++) {
-      cleanupDepEffect(effect.deps[i], effect)
+  if (effect._trackToken) {
+    const deps = depsMap.get(effect._trackToken)
+    if (deps) {
+      if (deps.length > effect._depsLength) {
+        for (let i = effect._depsLength; i < deps.length; i++) {
+          cleanupDepEffect(deps[i], effect)
+        }
+        deps.length = effect._depsLength
+      }
     }
-    effect.deps.length = effect._depsLength
   }
 }
 
@@ -281,12 +321,16 @@ export function trackEffect(
 ) {
   if (dep.get(effect._trackToken) !== effect._trackId) {
     dep.set(effect._trackToken, effect._trackId)
-    const oldDep = effect.deps[effect._depsLength]
+    let deps = depsMap.get(effect._trackToken)
+    if (!deps) {
+      depsMap.set(effect._trackToken, (deps = []))
+    }
+    const oldDep = deps[effect._depsLength]
     if (oldDep !== dep) {
       if (oldDep) {
         cleanupDepEffect(oldDep, effect)
       }
-      effect.deps[effect._depsLength++] = dep
+      deps[effect._depsLength++] = dep
     } else {
       effect._depsLength++
     }
@@ -304,12 +348,9 @@ export function triggerEffects(
   debuggerEventExtraInfo?: DebuggerEventExtraInfo
 ) {
   pauseScheduling()
-  let reclaimedTokens: WeakRef<ReactiveEffect>[] | undefined
   for (const trackToken of dep.keys()) {
     const effect = trackToken.deref()
     if (!effect) {
-      reclaimedTokens ??= []
-      reclaimedTokens.push(trackToken)
       continue
     }
     if (!effect.allowRecurse && effect._runnings) {
@@ -335,13 +376,5 @@ export function triggerEffects(
       }
     }
   }
-  if (reclaimedTokens) {
-    for (const token of reclaimedTokens) {
-      dep.delete(token)
-    }
-    if (dep.size === 0) {
-      dep.cleanup()
-    }
-  }
   resetScheduling()
 }