Skip to content

Commit 93b5beb

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

File tree

4 files changed

+83
-4
lines changed

4 files changed

+83
-4
lines changed

packages/reactivity/__tests__/effect.spec.ts

Lines changed: 70 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,68 @@ 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+
stop(runner)
1013+
expect(getDepFromReactive(toRaw(obj), 'prop')).toBeUndefined()
1014+
obj.prop = 3
1015+
runner()
1016+
expect(getDepFromReactive(toRaw(obj), 'prop')).toBeUndefined()
1017+
})
1018+
1019+
it('should only remove the dep when the last effect is stopped', () => {
1020+
const obj = reactive({ prop: 1 })
1021+
expect(getDepFromReactive(toRaw(obj), 'prop')).toBeUndefined()
1022+
const runner1 = effect(() => obj.prop)
1023+
const dep = getDepFromReactive(toRaw(obj), 'prop')
1024+
expect(dep).toHaveLength(1)
1025+
const runner2 = effect(() => obj.prop)
1026+
expect(getDepFromReactive(toRaw(obj), 'prop')).toBe(dep)
1027+
expect(dep).toHaveLength(2)
1028+
obj.prop = 2
1029+
expect(getDepFromReactive(toRaw(obj), 'prop')).toBe(dep)
1030+
expect(dep).toHaveLength(2)
1031+
stop(runner1)
1032+
expect(getDepFromReactive(toRaw(obj), 'prop')).toBe(dep)
1033+
expect(dep).toHaveLength(1)
1034+
obj.prop = 3
1035+
expect(getDepFromReactive(toRaw(obj), 'prop')).toBe(dep)
1036+
expect(dep).toHaveLength(1)
1037+
stop(runner2)
1038+
expect(getDepFromReactive(toRaw(obj), 'prop')).toBeUndefined()
1039+
obj.prop = 4
1040+
runner1()
1041+
runner2()
1042+
expect(getDepFromReactive(toRaw(obj), 'prop')).toBeUndefined()
1043+
})
1044+
1045+
it('should remove the dep when it is no longer used by the effect', () => {
1046+
const obj = reactive<{ a: number; b: number; c: 'a' | 'b' }>({
1047+
a: 1,
1048+
b: 2,
1049+
c: 'a'
1050+
})
1051+
expect(getDepFromReactive(toRaw(obj), 'prop')).toBeUndefined()
1052+
effect(() => obj[obj.c])
1053+
const depC = getDepFromReactive(toRaw(obj), 'c')
1054+
expect(getDepFromReactive(toRaw(obj), 'a')).toHaveLength(1)
1055+
expect(getDepFromReactive(toRaw(obj), 'b')).toBeUndefined()
1056+
expect(depC).toHaveLength(1)
1057+
obj.c = 'b'
1058+
obj.a = 4
1059+
expect(getDepFromReactive(toRaw(obj), 'a')).toBeUndefined()
1060+
expect(getDepFromReactive(toRaw(obj), 'b')).toHaveLength(1)
1061+
expect(getDepFromReactive(toRaw(obj), 'c')).toBe(depC)
1062+
expect(depC).toHaveLength(1)
1063+
})
1064+
})
9961065
})

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)