Skip to content

Commit

Permalink
fix(reactiivty): avoid unnecessary watcher effect removal from inacti…
Browse files Browse the repository at this point in the history
…ve scope

close #5783
close #5806
  • Loading branch information
yyx990803 committed Nov 14, 2024
1 parent e9f3e6b commit 2193284
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 2 deletions.
37 changes: 37 additions & 0 deletions packages/reactivity/__tests__/effectScope.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -322,4 +322,41 @@ describe('reactivity/effect/scope', () => {
scope.resume()
expect(fnSpy).toHaveBeenCalledTimes(3)
})

test('removing a watcher while stopping its effectScope', async () => {
const count = ref(0)
const scope = effectScope()
let watcherCalls = 0
let cleanupCalls = 0

scope.run(() => {
const stop1 = watch(count, () => {
watcherCalls++
})
watch(count, (val, old, onCleanup) => {
watcherCalls++
onCleanup(() => {
cleanupCalls++
stop1()
})
})
watch(count, () => {
watcherCalls++
})
})

expect(watcherCalls).toBe(0)
expect(cleanupCalls).toBe(0)

count.value++
await nextTick()
expect(watcherCalls).toBe(3)
expect(cleanupCalls).toBe(0)

scope.stop()
count.value++
await nextTick()
expect(watcherCalls).toBe(3)
expect(cleanupCalls).toBe(1)
})
})
2 changes: 1 addition & 1 deletion packages/reactivity/src/effectScope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ export class EffectScope {

stop(fromParent?: boolean): void {
if (this._active) {
this._active = false
let i, l
for (i = 0, l = this.effects.length; i < l; i++) {
this.effects[i].stop()
Expand All @@ -139,7 +140,6 @@ export class EffectScope {
}
}
this.parent = undefined
this._active = false
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/reactivity/src/watch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ export function watch(
const scope = getCurrentScope()
const watchHandle: WatchHandle = () => {
effect.stop()
if (scope) {
if (scope && scope.active) {
remove(scope.effects, effect)
}
}
Expand Down

0 comments on commit 2193284

Please sign in to comment.