Skip to content

Commit

Permalink
fix(reactivity): effect shoud only recursively self trigger with expl…
Browse files Browse the repository at this point in the history
…icit options

fix #2125
  • Loading branch information
yyx990803 committed Sep 16, 2020
1 parent 89e9ab8 commit 3810de7
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 6 deletions.
7 changes: 6 additions & 1 deletion packages/reactivity/src/effect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export interface ReactiveEffectOptions {
onTrack?: (event: DebuggerEvent) => void
onTrigger?: (event: DebuggerEvent) => void
onStop?: () => void
allowRecurse?: boolean
}

export type DebuggerEvent = {
Expand Down Expand Up @@ -178,7 +179,11 @@ export function trigger(
const effects = new Set<ReactiveEffect>()
const add = (effectsToAdd: Set<ReactiveEffect> | undefined) => {
if (effectsToAdd) {
effectsToAdd.forEach(effect => effects.add(effect))
effectsToAdd.forEach(effect => {
if (effect !== activeEffect || effect.options.allowRecurse) {
effects.add(effect)
}
})
}
}

Expand Down
13 changes: 13 additions & 0 deletions packages/runtime-core/__tests__/apiWatch.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -779,4 +779,17 @@ describe('api: watch', () => {
// should trigger now
expect(sideEffect).toBe(2)
})

// #2125
test('watchEffect should not recursively trigger itself', async () => {
const spy = jest.fn()
const price = ref(10)
const history = ref<number[]>([])
watchEffect(() => {
history.value.push(price.value)
spy()
})
await nextTick()
expect(spy).toHaveBeenCalledTimes(1)
})
})
35 changes: 34 additions & 1 deletion packages/runtime-core/__tests__/rendererComponent.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@ import {
nodeOps,
serializeInner,
nextTick,
VNode
VNode,
provide,
inject,
Ref
} from '@vue/runtime-test'

describe('renderer: component', () => {
Expand Down Expand Up @@ -104,4 +107,34 @@ describe('renderer: component', () => {
)
expect(Comp1.updated).not.toHaveBeenCalled()
})

// #2043
test('component child synchronously updating parent state should trigger parent re-render', async () => {
const App = {
setup() {
const n = ref(0)
provide('foo', n)
return () => {
return [h('div', n.value), h(Child)]
}
}
}

const Child = {
setup() {
const n = inject<Ref<number>>('foo')!
n.value++

return () => {
return h('div', n.value)
}
}
}

const root = nodeOps.createElement('div')
render(h(App), root)
expect(serializeInner(root)).toBe(`<div>0</div><div>1</div>`)
await nextTick()
expect(serializeInner(root)).toBe(`<div>1</div><div>1</div>`)
})
})
8 changes: 4 additions & 4 deletions packages/runtime-core/src/renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ import {
flushPostFlushCbs,
invalidateJob,
flushPreFlushCbs,
SchedulerJob,
SchedulerCb
} from './scheduler'
import { effect, stop, ReactiveEffectOptions, isRef } from '@vue/reactivity'
Expand Down Expand Up @@ -261,14 +260,17 @@ export const enum MoveType {
}

const prodEffectOptions = {
scheduler: queueJob
scheduler: queueJob,
// #1801, #2043 component render effects should allow recursive updates
allowRecurse: true
}

function createDevEffectOptions(
instance: ComponentInternalInstance
): ReactiveEffectOptions {
return {
scheduler: queueJob,
allowRecurse: true,
onTrack: instance.rtc ? e => invokeArrayFns(instance.rtc!, e) : void 0,
onTrigger: instance.rtg ? e => invokeArrayFns(instance.rtg!, e) : void 0
}
Expand Down Expand Up @@ -1489,8 +1491,6 @@ function baseCreateRenderer(
}
}
}, __DEV__ ? createDevEffectOptions(instance) : prodEffectOptions)
// #1801 mark it to allow recursive updates
;(instance.update as SchedulerJob).allowRecurse = true
}

const updateComponentPreRender = (
Expand Down

0 comments on commit 3810de7

Please sign in to comment.