Skip to content

Commit

Permalink
fix(scheduler): only allow watch callbacks to be self-triggering
Browse files Browse the repository at this point in the history
  • Loading branch information
yangmingshan committed Aug 5, 2020
1 parent d086cd9 commit 7ee769e
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 23 deletions.
46 changes: 46 additions & 0 deletions packages/wechat/__tests__/scheduler.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,5 +90,51 @@ describe('scheduler', () => {
} catch (error) {
expect(error).toBe(err)
}

// This one should no longer error
await nextTick()
})

test('should prevent self-triggering jobs by default', async () => {
let count = 0
const job = () => {
if (count < 3) {
count++
queueJob(job)
}
}

queueJob(job)
await nextTick()
// Only runs once - a job cannot queue itself
expect(count).toBe(1)
})

test('should allow watcher callbacks to trigger itself', async () => {
let count = 0
const job = () => {
if (count < 3) {
count++
queueJob(job)
}
}

job.cb = true
queueJob(job)
await nextTick()
expect(count).toBe(3)
})

test('should prevent duplicate queue', async () => {
let count = 0
const job = () => {
count++
}

job.cb = true
queueJob(job)
queueJob(job)
await nextTick()
expect(count).toBe(1)
})
})
66 changes: 45 additions & 21 deletions packages/wechat/src/scheduler.ts
Original file line number Diff line number Diff line change
@@ -1,24 +1,42 @@
export type Job = () => void
export interface SchedulerJob {
/**
* Indicates this is a watch() callback and is allowed to trigger itself.
* A watch callback doesn't track its dependencies so if it triggers itself
* again, it's likely intentional and it is the user's responsibility to
* perform recursive state mutation that eventually stabilizes.
*/
cb?: boolean
(): void
}

const queue: Job[] = []
const queue: SchedulerJob[] = []
const resolvedPromise: Promise<any> = Promise.resolve()
let currentFlushPromise: Promise<void> | null = null

let isFlushing = false
let isFlushPending = false
let flushIndex = -1
let flushIndex = 0

const RECURSION_LIMIT = 100
type CountMap = Map<Job | Function, number>
type CountMap = Map<SchedulerJob | Function, number>

export function nextTick(fn?: () => void): Promise<void> {
const p = currentFlushPromise || resolvedPromise
// eslint-disable-next-line promise/prefer-await-to-then
return fn ? p.then(fn) : p
}

export function queueJob(job: Job): void {
if (!queue.includes(job, flushIndex + 1)) {
export function queueJob(job: SchedulerJob) {
// The dedupe search uses the startIndex argument of Array.includes()
// by default the search index includes the current job that is being run
// so it cannot recursively trigger itself again.
// if the job is a watch() callback, the search will start with a +1 index to
// allow it recursively trigger itself - it is the user's responsibility to
// ensure it doesn't end up in an infinite loop.
if (
queue.length === 0 ||
!queue.includes(job, isFlushing && job.cb ? flushIndex + 1 : flushIndex)
) {
queue.push(job)
queueFlush()
}
Expand All @@ -40,30 +58,36 @@ function flushJobs(seen?: CountMap): void {
seen = seen || new Map()
}

for (flushIndex = 0; flushIndex < queue.length; flushIndex++) {
const job = queue[flushIndex]
/* istanbul ignore else */
if (__DEV__) {
checkRecursiveUpdates(seen!, job)
try {
for (flushIndex = 0; flushIndex < queue.length; flushIndex++) {
const job = queue[flushIndex]
/* istanbul ignore else */
if (__DEV__) {
checkRecursiveUpdates(seen!, job)
}

job()
}
} finally {
flushIndex = 0
queue.length = 0

job()
isFlushing = false
currentFlushPromise = null
}

flushIndex = -1
queue.length = 0

isFlushing = false
currentFlushPromise = null
}

function checkRecursiveUpdates(seen: CountMap, fn: Job | Function): void {
function checkRecursiveUpdates(
seen: CountMap,
fn: SchedulerJob | Function
): void {
const count = seen.get(fn) || 0
/* istanbul ignore if */
if (count > RECURSION_LIMIT) {
throw new Error(
'Maximum recursive updates exceeded. ' +
'You may have code that is mutating state in your watcher source function. '
`Maximum recursive updates exceeded. ` +
`This means you have a reactive effect that is mutating its own ` +
`dependencies and thus recursively triggering itself.`
)
} else {
seen.set(fn, count + 1)
Expand Down
8 changes: 6 additions & 2 deletions packages/wechat/src/watch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
ReactiveEffectOptions,
isReactive,
} from '@vue/reactivity'
import { queueJob } from './scheduler'
import { queueJob, SchedulerJob } from './scheduler'
import { recordInstanceBoundEffect } from './computed'
import { getCurrentInstance } from './instance'
import { isArray, isObject, isFunction, hasChanged, remove } from './utils'
Expand Down Expand Up @@ -208,7 +208,7 @@ function doWatch(
}

let oldValue = isArray(source) ? [] : INITIAL_WATCHER_VALUE
const job = () => {
const job: SchedulerJob = () => {
if (!runner.active) {
return
}
Expand Down Expand Up @@ -236,6 +236,10 @@ function doWatch(
}
}

// Important: mark the job as a watcher callback so that scheduler knows it
// is allowed to self-trigger
job.cb = Boolean(cb)

let scheduler: (job: () => any) => void
if (flush === 'sync') {
scheduler = job
Expand Down

0 comments on commit 7ee769e

Please sign in to comment.