fix(reactivity): fix property dep removal regression

close #12020
close #12021
This commit is contained in:
Evan You 2024-09-26 16:58:38 +08:00
parent c0e9434414
commit 6001e5c81a
No known key found for this signature in database
GPG Key ID: 00E9AB7A6704CE0A
4 changed files with 78 additions and 38 deletions

View File

@ -1023,6 +1023,7 @@ describe('reactivity/computed', () => {
expect(p.value).toBe(3) expect(p.value).toBe(3)
}) })
// #11995
test('computed dep cleanup should not cause property dep to be deleted', () => { test('computed dep cleanup should not cause property dep to be deleted', () => {
const toggle = ref(true) const toggle = ref(true)
const state = reactive({ a: 1 }) const state = reactive({ a: 1 })
@ -1037,4 +1038,23 @@ describe('reactivity/computed', () => {
state.a++ state.a++
expect(pp.value).toBe(2) expect(pp.value).toBe(2)
}) })
// #12020
test('computed value updates correctly after dep cleanup', () => {
const obj = reactive({ foo: 1, flag: 1 })
const c1 = computed(() => obj.foo)
let foo
effect(() => {
foo = obj.flag ? (obj.foo, c1.value) : 0
})
expect(foo).toBe(1)
obj.flag = 0
expect(foo).toBe(0)
obj.foo = 2
obj.flag = 1
expect(foo).toBe(2)
})
}) })

View File

@ -13,6 +13,7 @@ import {
} from '../src/reactive' } from '../src/reactive'
import { computed } from '../src/computed' import { computed } from '../src/computed'
import { effect } from '../src/effect' import { effect } from '../src/effect'
import { targetMap } from '../src/dep'
describe('reactivity/reactive', () => { describe('reactivity/reactive', () => {
test('Object', () => { test('Object', () => {
@ -398,4 +399,14 @@ describe('reactivity/reactive', () => {
a.value++ a.value++
}).not.toThrow() }).not.toThrow()
}) })
// #11979
test('should release property Dep instance if it no longer has subscribers', () => {
let obj = { x: 1 }
let a = reactive(obj)
const e = effect(() => a.x)
expect(targetMap.get(obj)?.get('x')).toBeTruthy()
e.effect.stop()
expect(targetMap.get(obj)?.get('x')).toBeFalsy()
})
}) })

View File

@ -89,6 +89,11 @@ export class Dep {
map?: KeyToDepMap = undefined map?: KeyToDepMap = undefined
key?: unknown = undefined key?: unknown = undefined
/**
* Subscriber counter
*/
sc: number = 0
constructor(public computed?: ComputedRefImpl | undefined) { constructor(public computed?: ComputedRefImpl | undefined) {
if (__DEV__) { if (__DEV__) {
this.subsHead = undefined this.subsHead = undefined
@ -113,9 +118,7 @@ export class Dep {
activeSub.depsTail = link activeSub.depsTail = link
} }
if (activeSub.flags & EffectFlags.TRACKING) {
addSub(link) addSub(link)
}
} else if (link.version === -1) { } else if (link.version === -1) {
// reused from last run - already a sub, just sync version // reused from last run - already a sub, just sync version
link.version = this.version link.version = this.version
@ -197,6 +200,8 @@ export class Dep {
} }
function addSub(link: Link) { function addSub(link: Link) {
link.dep.sc++
if (link.sub.flags & EffectFlags.TRACKING) {
const computed = link.dep.computed const computed = link.dep.computed
// computed getting its first subscriber // computed getting its first subscriber
// enable tracking + lazily subscribe to all its deps // enable tracking + lazily subscribe to all its deps
@ -218,6 +223,7 @@ function addSub(link: Link) {
} }
link.dep.subs = link link.dep.subs = link
}
} }
// The main WeakMap that stores {target -> key -> dep} connections. // The main WeakMap that stores {target -> key -> dep} connections.

View File

@ -1,7 +1,7 @@
import { extend, hasChanged } from '@vue/shared' import { extend, hasChanged } from '@vue/shared'
import type { ComputedRefImpl } from './computed' import type { ComputedRefImpl } from './computed'
import type { TrackOpTypes, TriggerOpTypes } from './constants' import type { TrackOpTypes, TriggerOpTypes } from './constants'
import { type Link, globalVersion, targetMap } from './dep' import { type Link, globalVersion } from './dep'
import { activeEffectScope } from './effectScope' import { activeEffectScope } from './effectScope'
import { warn } from './warning' import { warn } from './warning'
@ -292,7 +292,7 @@ function prepareDeps(sub: Subscriber) {
} }
} }
function cleanupDeps(sub: Subscriber, fromComputed = false) { function cleanupDeps(sub: Subscriber) {
// Cleanup unsued deps // Cleanup unsued deps
let head let head
let tail = sub.depsTail let tail = sub.depsTail
@ -302,7 +302,7 @@ function cleanupDeps(sub: Subscriber, fromComputed = false) {
if (link.version === -1) { if (link.version === -1) {
if (link === tail) tail = prev if (link === tail) tail = prev
// unused - remove it from the dep's subscribing effect list // unused - remove it from the dep's subscribing effect list
removeSub(link, fromComputed) removeSub(link)
// also remove it from this effect's dep list // also remove it from this effect's dep list
removeDep(link) removeDep(link)
} else { } else {
@ -394,12 +394,12 @@ export function refreshComputed(computed: ComputedRefImpl): undefined {
} finally { } finally {
activeSub = prevSub activeSub = prevSub
shouldTrack = prevShouldTrack shouldTrack = prevShouldTrack
cleanupDeps(computed, true) cleanupDeps(computed)
computed.flags &= ~EffectFlags.RUNNING computed.flags &= ~EffectFlags.RUNNING
} }
} }
function removeSub(link: Link, fromComputed = false) { function removeSub(link: Link, soft = false) {
const { dep, prevSub, nextSub } = link const { dep, prevSub, nextSub } = link
if (prevSub) { if (prevSub) {
prevSub.nextSub = nextSub prevSub.nextSub = nextSub
@ -418,20 +418,23 @@ function removeSub(link: Link, fromComputed = false) {
dep.subsHead = nextSub dep.subsHead = nextSub
} }
if (!dep.subs) { if (!dep.subs && dep.computed) {
// last subscriber removed
if (dep.computed) {
// if computed, unsubscribe it from all its deps so this computed and its // if computed, unsubscribe it from all its deps so this computed and its
// value can be GCed // value can be GCed
dep.computed.flags &= ~EffectFlags.TRACKING dep.computed.flags &= ~EffectFlags.TRACKING
for (let l = dep.computed.deps; l; l = l.nextDep) { for (let l = dep.computed.deps; l; l = l.nextDep) {
// here we are only "soft" unsubscribing because the computed still keeps
// referencing the deps and the dep should not decrease its sub count
removeSub(l, true) removeSub(l, true)
} }
} else if (dep.map && !fromComputed) {
// property dep, remove it from the owner depsMap
dep.map.delete(dep.key)
if (!dep.map.size) targetMap.delete(dep.target!)
} }
if (!soft && !--dep.sc && dep.map) {
// #11979
// property dep no longer has effect subscribers, delete it
// this mostly is for the case where an object is kept in memory but only a
// subset of its properties is tracked at one time
dep.map.delete(dep.key)
} }
} }