Skip to content

Commit

Permalink
fix(reactivity): dereference nested effect scopes on manual stop
Browse files Browse the repository at this point in the history
  • Loading branch information
yyx990803 committed Jul 28, 2021
1 parent da6c055 commit 1867591
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 2 deletions.
8 changes: 8 additions & 0 deletions packages/reactivity/__tests__/effectScope.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,14 @@ describe('reactivity/effect/scope', () => {
expect(dummy).toBe(7)
})

it('should derefence child scope from parent scope after stopping child scope (no memleaks)', async () => {
const parent = new EffectScope()
const child = parent.run(() => new EffectScope())!
expect(parent.effects.includes(child)).toBe(true)
child.stop()
expect(parent.effects.includes(child)).toBe(false)
})

it('test with higher level APIs', async () => {
const r = ref(1)

Expand Down
10 changes: 8 additions & 2 deletions packages/reactivity/src/effectScope.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { remove } from '@vue/shared'
import { ReactiveEffect } from './effect'
import { warn } from './warning'

Expand All @@ -8,10 +9,12 @@ export class EffectScope {
active = true
effects: (ReactiveEffect | EffectScope)[] = []
cleanups: (() => void)[] = []
parent: EffectScope | undefined

constructor(detached = false) {
if (!detached) {
recordEffectScope(this)
this.parent = activeEffectScope
}
}

Expand Down Expand Up @@ -42,11 +45,14 @@ export class EffectScope {
}
}

stop() {
stop(fromParent = false) {
if (this.active) {
this.effects.forEach(e => e.stop())
this.effects.forEach(e => e.stop(true))
this.cleanups.forEach(cleanup => cleanup())
this.active = false
if (!fromParent && this.parent) {
remove(this.parent.effects, this)
}
}
}
}
Expand Down

0 comments on commit 1867591

Please sign in to comment.