Skip to content

Commit

Permalink
fix(reactivity): scheduled effect should not execute if stopped
Browse files Browse the repository at this point in the history
fix #910
  • Loading branch information
yyx990803 committed Apr 2, 2020
1 parent 5c33776 commit 0764c33
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 22 deletions.
22 changes: 22 additions & 0 deletions packages/reactivity/__tests__/effect.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -678,6 +678,28 @@ describe('reactivity/effect', () => {
expect(dummy).toBe(3)
})

it('stop with scheduler', () => {
let dummy
const obj = reactive({ prop: 1 })
const queue: (() => void)[] = []
const runner = effect(
() => {
dummy = obj.prop
},
{
scheduler: e => queue.push(e)
}
)
obj.prop = 2
expect(dummy).toBe(1)
expect(queue.length).toBe(1)
stop(runner)

// a scheduled effect should not execute anymore after stopped
queue.forEach(e => e())
expect(dummy).toBe(1)
})

it('events: onStop', () => {
const onStop = jest.fn()
const runner = effect(() => {}, {
Expand Down
40 changes: 18 additions & 22 deletions packages/reactivity/src/effect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ type KeyToDepMap = Map<any, Dep>
const targetMap = new WeakMap<any, KeyToDepMap>()

export interface ReactiveEffect<T = any> {
(): T
(...args: any[]): T
_isEffect: true
active: boolean
raw: () => T
Expand Down Expand Up @@ -75,11 +75,26 @@ export function stop(effect: ReactiveEffect) {
}

function createReactiveEffect<T = any>(
fn: () => T,
fn: (...args: any[]) => T,
options: ReactiveEffectOptions
): ReactiveEffect<T> {
const effect = function reactiveEffect(...args: unknown[]): unknown {
return run(effect, fn, args)
if (!effect.active) {
return options.scheduler ? undefined : fn(...args)
}
if (!effectStack.includes(effect)) {
cleanup(effect)
try {
enableTracking()
effectStack.push(effect)
activeEffect = effect
return fn(...args)
} finally {
effectStack.pop()
resetTracking()
activeEffect = effectStack[effectStack.length - 1]
}
}
} as ReactiveEffect
effect._isEffect = true
effect.active = true
Expand All @@ -89,25 +104,6 @@ function createReactiveEffect<T = any>(
return effect
}

function run(effect: ReactiveEffect, fn: Function, args: unknown[]): unknown {
if (!effect.active) {
return fn(...args)
}
if (!effectStack.includes(effect)) {
cleanup(effect)
try {
enableTracking()
effectStack.push(effect)
activeEffect = effect
return fn(...args)
} finally {
effectStack.pop()
resetTracking()
activeEffect = effectStack[effectStack.length - 1]
}
}
}

function cleanup(effect: ReactiveEffect) {
const { deps } = effect
if (deps.length) {
Expand Down

0 comments on commit 0764c33

Please sign in to comment.