Skip to content

Commit 790a51a

Browse files
committed
fix(reactivity): fix dep memory leak
Co-Authored-By: skirtle <[email protected]> vuejs#7827
1 parent 50418be commit 790a51a

File tree

4 files changed

+84
-4
lines changed

4 files changed

+84
-4
lines changed

packages/reactivity/__tests__/effect.spec.ts

Lines changed: 71 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,12 @@ import {
1111
readonly,
1212
ReactiveEffectRunner
1313
} from '../src/index'
14-
import { ITERATE_KEY, pauseScheduling, resetScheduling } from '../src/effect'
14+
import {
15+
ITERATE_KEY,
16+
getDepFromReactive,
17+
pauseScheduling,
18+
resetScheduling
19+
} from '../src/effect'
1520

1621
describe('reactivity/effect', () => {
1722
it('should run the passed function once (wrapped by a effect)', () => {
@@ -993,4 +998,69 @@ describe('reactivity/effect', () => {
993998
resetScheduling()
994999
expect(counterSpy).toHaveBeenCalledTimes(1)
9951000
})
1001+
1002+
describe('empty dep cleanup', () => {
1003+
it('should remove the dep when the effect is stopped', () => {
1004+
const obj = reactive({ prop: 1 })
1005+
expect(getDepFromReactive(toRaw(obj), 'prop')).toBeUndefined()
1006+
const runner = effect(() => obj.prop)
1007+
const dep = getDepFromReactive(toRaw(obj), 'prop')
1008+
expect(dep).toHaveLength(1)
1009+
obj.prop = 2
1010+
expect(getDepFromReactive(toRaw(obj), 'prop')).toBe(dep)
1011+
expect(dep).toHaveLength(1)
1012+
debugger
1013+
stop(runner)
1014+
expect(getDepFromReactive(toRaw(obj), 'prop')).toBeUndefined()
1015+
obj.prop = 3
1016+
runner()
1017+
expect(getDepFromReactive(toRaw(obj), 'prop')).toBeUndefined()
1018+
})
1019+
1020+
it('should only remove the dep when the last effect is stopped', () => {
1021+
const obj = reactive({ prop: 1 })
1022+
expect(getDepFromReactive(toRaw(obj), 'prop')).toBeUndefined()
1023+
const runner1 = effect(() => obj.prop)
1024+
const dep = getDepFromReactive(toRaw(obj), 'prop')
1025+
expect(dep).toHaveLength(1)
1026+
const runner2 = effect(() => obj.prop)
1027+
expect(getDepFromReactive(toRaw(obj), 'prop')).toBe(dep)
1028+
expect(dep).toHaveLength(2)
1029+
obj.prop = 2
1030+
expect(getDepFromReactive(toRaw(obj), 'prop')).toBe(dep)
1031+
expect(dep).toHaveLength(2)
1032+
stop(runner1)
1033+
expect(getDepFromReactive(toRaw(obj), 'prop')).toBe(dep)
1034+
expect(dep).toHaveLength(1)
1035+
obj.prop = 3
1036+
expect(getDepFromReactive(toRaw(obj), 'prop')).toBe(dep)
1037+
expect(dep).toHaveLength(1)
1038+
stop(runner2)
1039+
expect(getDepFromReactive(toRaw(obj), 'prop')).toBeUndefined()
1040+
obj.prop = 4
1041+
runner1()
1042+
runner2()
1043+
expect(getDepFromReactive(toRaw(obj), 'prop')).toBeUndefined()
1044+
})
1045+
1046+
it('should remove the dep when it is no longer used by the effect', () => {
1047+
const obj = reactive<{ a: number; b: number; c: 'a' | 'b' }>({
1048+
a: 1,
1049+
b: 2,
1050+
c: 'a'
1051+
})
1052+
expect(getDepFromReactive(toRaw(obj), 'prop')).toBeUndefined()
1053+
effect(() => obj[obj.c])
1054+
const depC = getDepFromReactive(toRaw(obj), 'c')
1055+
expect(getDepFromReactive(toRaw(obj), 'a')).toHaveLength(1)
1056+
expect(getDepFromReactive(toRaw(obj), 'b')).toBeUndefined()
1057+
expect(depC).toHaveLength(1)
1058+
obj.c = 'b'
1059+
obj.a = 4
1060+
expect(getDepFromReactive(toRaw(obj), 'a')).toBeUndefined()
1061+
expect(getDepFromReactive(toRaw(obj), 'b')).toHaveLength(1)
1062+
expect(getDepFromReactive(toRaw(obj), 'c')).toBe(depC)
1063+
expect(depC).toHaveLength(1)
1064+
})
1065+
})
9961066
})

packages/reactivity/src/dep.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,16 @@ import type { ReactiveEffect } from './effect'
22
import type { ComputedRefImpl } from './computed'
33

44
export type Dep = Map<ReactiveEffect, number> & {
5+
cleanup: () => void
56
computed?: ComputedRefImpl<any>
67
}
78

8-
export const createDep = (computed?: ComputedRefImpl<any>): Dep => {
9-
const dep: Dep = new Map()
9+
export const createDep = (
10+
cleanup: () => void,
11+
computed?: ComputedRefImpl<any>
12+
): Dep => {
13+
const dep: Dep = new Map() as Dep
14+
dep.cleanup = cleanup
1015
dep.computed = computed
1116
return dep
1217
}

packages/reactivity/src/effect.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,9 @@ function cleanupDepEffect(dep: Dep, effect: ReactiveEffect) {
144144
const trackId = dep.get(effect)
145145
if (trackId !== undefined && effect._trackId !== trackId) {
146146
dep.delete(effect)
147+
if (dep.size === 0) {
148+
dep.cleanup()
149+
}
147150
}
148151
}
149152

@@ -275,7 +278,7 @@ export function track(target: object, type: TrackOpTypes, key: unknown) {
275278
}
276279
let dep = depsMap.get(key)
277280
if (!dep) {
278-
depsMap.set(key, (dep = createDep()))
281+
depsMap.set(key, (dep = createDep(() => depsMap!.delete(key))))
279282
}
280283
if (__DEV__) {
281284
trackEffect(activeEffect, dep, {

packages/reactivity/src/ref.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ export function trackRefValue(ref: RefBase<any>) {
4646
activeEffect,
4747
ref.dep ||
4848
(ref.dep = createDep(
49+
() => (ref.dep = undefined),
4950
ref instanceof ComputedRefImpl ? ref : undefined
5051
)),
5152
{
@@ -59,6 +60,7 @@ export function trackRefValue(ref: RefBase<any>) {
5960
activeEffect,
6061
ref.dep ||
6162
(ref.dep = createDep(
63+
() => (ref.dep = undefined),
6264
ref instanceof ComputedRefImpl ? ref : undefined
6365
))
6466
)

0 commit comments

Comments
 (0)