Skip to content

Commit

Permalink
fix(watch): ensure watchers respect detached scope
Browse files Browse the repository at this point in the history
fix #4158
  • Loading branch information
yyx990803 committed Jul 20, 2021
1 parent 2bdee50 commit bc7f976
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 15 deletions.
31 changes: 27 additions & 4 deletions packages/runtime-core/__tests__/apiWatch.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ import {
TriggerOpTypes,
triggerRef,
shallowRef,
Ref
Ref,
effectScope
} from '@vue/reactivity'
import { watchPostEffect } from '../src/apiWatch'

Expand Down Expand Up @@ -848,7 +849,7 @@ describe('api: watch', () => {
})

// https://github.com/vuejs/vue-next/issues/2381
test('$watch should always register its effects with itw own instance', async () => {
test('$watch should always register its effects with its own instance', async () => {
let instance: ComponentInternalInstance | null
let _show: Ref<boolean>

Expand Down Expand Up @@ -889,14 +890,14 @@ describe('api: watch', () => {
expect(instance!).toBeDefined()
expect(instance!.scope.effects).toBeInstanceOf(Array)
// includes the component's own render effect AND the watcher effect
expect(instance!.scope.effects!.length).toBe(2)
expect(instance!.scope.effects.length).toBe(2)

_show!.value = false

await nextTick()
await nextTick()

expect(instance!.scope.effects![0].active).toBe(false)
expect(instance!.scope.effects[0].active).toBe(false)
})

test('this.$watch should pass `this.proxy` to watch source as the first argument ', () => {
Expand Down Expand Up @@ -1024,4 +1025,26 @@ describe('api: watch', () => {
expect(plus.value).toBe(true)
expect(count).toBe(0)
})

// #4158
test('watch should not register in owner component if created inside detached scope', () => {
let instance: ComponentInternalInstance
const Comp = {
setup() {
instance = getCurrentInstance()!
effectScope(true).run(() => {
watch(
() => 1,
() => {}
)
})
return () => ''
}
}
const root = nodeOps.createElement('div')
createApp(Comp).mount(root)
// should not record watcher in detached scope and only the instance's
// own update effect
expect(instance!.scope.effects.length).toBe(1)
})
})
25 changes: 17 additions & 8 deletions packages/runtime-core/src/apiWatch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ import {
import {
currentInstance,
ComponentInternalInstance,
isInSSRComponentSetup
isInSSRComponentSetup,
setCurrentInstance,
unsetCurrentInstance
} from './component'
import {
ErrorCodes,
Expand Down Expand Up @@ -157,8 +159,7 @@ export function watch<T = any, Immediate extends Readonly<boolean> = false>(
function doWatch(
source: WatchSource | WatchSource[] | WatchEffect | object,
cb: WatchCallback | null,
{ immediate, deep, flush, onTrack, onTrigger }: WatchOptions = EMPTY_OBJ,
instance = currentInstance
{ immediate, deep, flush, onTrack, onTrigger }: WatchOptions = EMPTY_OBJ
): WatchStopHandle {
if (__DEV__ && !cb) {
if (immediate !== undefined) {
Expand All @@ -184,6 +185,7 @@ function doWatch(
)
}

const instance = currentInstance
let getter: () => any
let forceTrigger = false
let isMultiSource = false
Expand Down Expand Up @@ -340,8 +342,7 @@ function doWatch(
}
}

const scope = instance && instance.scope
const effect = new ReactiveEffect(getter, scheduler, scope)
const effect = new ReactiveEffect(getter, scheduler)

if (__DEV__) {
effect.onTrack = onTrack
Expand All @@ -366,8 +367,8 @@ function doWatch(

return () => {
effect.stop()
if (scope) {
remove(scope.effects!, effect)
if (instance && instance.scope) {
remove(instance.scope.effects!, effect)
}
}
}
Expand All @@ -392,7 +393,15 @@ export function instanceWatch(
cb = value.handler as Function
options = value
}
return doWatch(getter, cb.bind(publicThis), options, this)
const cur = currentInstance
setCurrentInstance(this)
const res = doWatch(getter, cb.bind(publicThis), options)
if (cur) {
setCurrentInstance(cur)
} else {
unsetCurrentInstance()
}
return res
}

export function createPathGetter(ctx: any, path: string) {
Expand Down
5 changes: 2 additions & 3 deletions packages/runtime-core/src/renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2304,9 +2304,8 @@ function baseCreateRenderer(
instance.emit('hook:beforeDestroy')
}

if (scope) {
scope.stop()
}
// stop effects in component scope
scope.stop()

// update may be null if a component is unmounted before its async
// setup has resolved.
Expand Down

0 comments on commit bc7f976

Please sign in to comment.