From 611437a3fe5e50a5a6f79e2f8a0ed59e74539626 Mon Sep 17 00:00:00 2001 From: Evan You Date: Thu, 13 Aug 2020 17:27:14 -0400 Subject: [PATCH] fix(runtime-core/scheduler): allow component render functions to trigger itself fix #1801 --- .../runtime-core/__tests__/scheduler.spec.ts | 6 ++--- packages/runtime-core/src/apiWatch.ts | 2 +- packages/runtime-core/src/renderer.ts | 5 +++- packages/runtime-core/src/scheduler.ts | 23 +++++++++++++------ 4 files changed, 24 insertions(+), 12 deletions(-) diff --git a/packages/runtime-core/__tests__/scheduler.spec.ts b/packages/runtime-core/__tests__/scheduler.spec.ts index 59855a69179..1811ce6b390 100644 --- a/packages/runtime-core/__tests__/scheduler.spec.ts +++ b/packages/runtime-core/__tests__/scheduler.spec.ts @@ -451,7 +451,7 @@ describe('scheduler', () => { expect(count).toBe(1) }) - test('should allow watcher callbacks to trigger itself', async () => { + test('should allow explicitly marked jobs to trigger itself', async () => { // normal job let count = 0 const job = () => { @@ -460,7 +460,7 @@ describe('scheduler', () => { queueJob(job) } } - job.cb = true + job.allowRecurse = true queueJob(job) await nextTick() expect(count).toBe(3) @@ -472,7 +472,7 @@ describe('scheduler', () => { queuePostFlushCb(cb) } } - cb.cb = true + cb.allowRecurse = true queuePostFlushCb(cb) await nextTick() expect(count).toBe(5) diff --git a/packages/runtime-core/src/apiWatch.ts b/packages/runtime-core/src/apiWatch.ts index cdc3df48b50..ac797b89ea3 100644 --- a/packages/runtime-core/src/apiWatch.ts +++ b/packages/runtime-core/src/apiWatch.ts @@ -261,7 +261,7 @@ function doWatch( // important: mark the job as a watcher callback so that scheduler knows it // it is allowed to self-trigger (#1727) - job.cb = !!cb + job.allowRecurse = !!cb let scheduler: (job: () => any) => void if (flush === 'sync') { diff --git a/packages/runtime-core/src/renderer.ts b/packages/runtime-core/src/renderer.ts index 8627cb00fea..cb1e3869345 100644 --- a/packages/runtime-core/src/renderer.ts +++ b/packages/runtime-core/src/renderer.ts @@ -41,7 +41,8 @@ import { queuePostFlushCb, flushPostFlushCbs, invalidateJob, - flushPreFlushCbs + flushPreFlushCbs, + SchedulerJob } from './scheduler' import { effect, stop, ReactiveEffectOptions, isRef } from '@vue/reactivity' import { updateProps } from './componentProps' @@ -1429,6 +1430,8 @@ function baseCreateRenderer( } } }, __DEV__ ? createDevEffectOptions(instance) : prodEffectOptions) + // #1801 mark it to allow recursive updates + ;(instance.update as SchedulerJob).allowRecurse = true } const updateComponentPreRender = ( diff --git a/packages/runtime-core/src/scheduler.ts b/packages/runtime-core/src/scheduler.ts index c9c0e47e483..1f168b7363e 100644 --- a/packages/runtime-core/src/scheduler.ts +++ b/packages/runtime-core/src/scheduler.ts @@ -8,12 +8,18 @@ export interface SchedulerJob { */ id?: number /** - * 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. + * Indicates whether the job is allowed to recursively trigger itself. + * By default, a job cannot trigger itself because some built-in method calls, + * e.g. Array.prototype.push actually performs reads as well (#1740) which + * can lead to confusing infinite loops. + * The allowed cases are component render functions and watch callbacks. + * Render functions may update child component props, which in turn trigger + * flush: "pre" watch callbacks that mutates state that the parent relies on + * (#1801). Watch callbacks 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 (#1727). */ - cb?: boolean + allowRecurse?: boolean } let isFlushing = false @@ -54,7 +60,7 @@ export function queueJob(job: SchedulerJob) { (!queue.length || !queue.includes( job, - isFlushing && job.cb ? flushIndex + 1 : flushIndex + isFlushing && job.allowRecurse ? flushIndex + 1 : flushIndex )) && job !== currentPreFlushParentJob ) { @@ -86,7 +92,10 @@ function queueCb( if (!isArray(cb)) { if ( !activeQueue || - !activeQueue.includes(cb, (cb as SchedulerJob).cb ? index + 1 : index) + !activeQueue.includes( + cb, + (cb as SchedulerJob).allowRecurse ? index + 1 : index + ) ) { pendingQueue.push(cb) }