fix(watch): flush:pre watchers should not fire if state change causes

owner component to unmount

fix #2291

packages/runtime-core/src/apiWatch.ts

@@ -9,7 +9,7 @@ import {
   EffectScheduler,
   DebuggerOptions
 } from '@vue/reactivity'
-import { SchedulerJob, queuePreFlushCb } from './scheduler'
+import { SchedulerJob, queueJob } from './scheduler'
 import {
   EMPTY_OBJ,
   isObject,
@@ -345,7 +345,9 @@ function doWatch(
     scheduler = () => queuePostRenderEffect(job, instance && instance.suspense)
   } else {
     // default: 'pre'
-    scheduler = () => queuePreFlushCb(job)
+    job.pre = true
+    if (instance) job.id = instance.uid
+    scheduler = () => queueJob(job)
   }
 
   const effect = new ReactiveEffect(getter, scheduler)

packages/runtime-core/src/renderer.ts

@@ -1590,7 +1590,7 @@ function baseCreateRenderer(
     pauseTracking()
     // props update may have triggered pre-flush watchers.
     // flush them before the render update.
-    flushPreFlushCbs(undefined, instance.update)
+    flushPreFlushCbs()
     resetTracking()
   }
 
@@ -2331,6 +2331,7 @@ function baseCreateRenderer(
     } else {
       patch(container._vnode || null, vnode, container, null, null, null, isSVG)
     }
+    flushPreFlushCbs()
     flushPostFlushCbs()
     container._vnode = vnode
   }

packages/runtime-core/src/scheduler.ts

@@ -5,6 +5,7 @@ import { warn } from './warning'
 
 export interface SchedulerJob extends Function {
   id?: number
+  pre?: boolean
   active?: boolean
   computed?: boolean
   /**
@@ -39,10 +40,6 @@ let isFlushPending = false
 const queue: SchedulerJob[] = []
 let flushIndex = 0
 
-const pendingPreFlushCbs: SchedulerJob[] = []
-let activePreFlushCbs: SchedulerJob[] | null = null
-let preFlushIndex = 0
-
 const pendingPostFlushCbs: SchedulerJob[] = []
 let activePostFlushCbs: SchedulerJob[] | null = null
 let postFlushIndex = 0
@@ -50,8 +47,6 @@ let postFlushIndex = 0
 const resolvedPromise = /*#__PURE__*/ Promise.resolve() as Promise<any>
 let currentFlushPromise: Promise<void> | null = null
 
-let currentPreFlushParentJob: SchedulerJob | null = null
-
 const RECURSION_LIMIT = 100
 type CountMap = Map<SchedulerJob, number>
 
@@ -89,12 +84,11 @@ export function queueJob(job: SchedulerJob) {
   // 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 ||
-      !queue.includes(
-        job,
-        isFlushing && job.allowRecurse ? flushIndex + 1 : flushIndex
-      )) &&
-    job !== currentPreFlushParentJob
+    !queue.length ||
+    !queue.includes(
+      job,
+      isFlushing && job.allowRecurse ? flushIndex + 1 : flushIndex
+    )
   ) {
     if (job.id == null) {
       queue.push(job)
@@ -119,71 +113,44 @@ export function invalidateJob(job: SchedulerJob) {
   }
 }
 
-function queueCb(
-  cb: SchedulerJobs,
-  activeQueue: SchedulerJob[] | null,
-  pendingQueue: SchedulerJob[],
-  index: number
-) {
+export function queuePostFlushCb(cb: SchedulerJobs) {
   if (!isArray(cb)) {
     if (
-      !activeQueue ||
-      !activeQueue.includes(cb, cb.allowRecurse ? index + 1 : index)
+      !activePostFlushCbs ||
+      !activePostFlushCbs.includes(
+        cb,
+        cb.allowRecurse ? postFlushIndex + 1 : postFlushIndex
+      )
     ) {
-      pendingQueue.push(cb)
+      pendingPostFlushCbs.push(cb)
     }
   } else {
     // if cb is an array, it is a component lifecycle hook which can only be
     // triggered by a job, which is already deduped in the main queue, so
     // we can skip duplicate check here to improve perf
-    pendingQueue.push(...cb)
+    pendingPostFlushCbs.push(...cb)
   }
   queueFlush()
 }
 
-export function queuePreFlushCb(cb: SchedulerJob) {
-  queueCb(cb, activePreFlushCbs, pendingPreFlushCbs, preFlushIndex)
-}
-
-export function queuePostFlushCb(cb: SchedulerJobs) {
-  queueCb(cb, activePostFlushCbs, pendingPostFlushCbs, postFlushIndex)
-}
-
-export function flushPreFlushCbs(
-  seen?: CountMap,
-  parentJob: SchedulerJob | null = null
-) {
-  if (pendingPreFlushCbs.length) {
-    currentPreFlushParentJob = parentJob
-    activePreFlushCbs = [...new Set(pendingPreFlushCbs)]
-    pendingPreFlushCbs.length = 0
-    if (__DEV__) {
-      seen = seen || new Map()
-    }
-    for (
-      preFlushIndex = 0;
-      preFlushIndex < activePreFlushCbs.length;
-      preFlushIndex++
-    ) {
-      if (
-        __DEV__ &&
-        checkRecursiveUpdates(seen!, activePreFlushCbs[preFlushIndex])
-      ) {
+export function flushPreFlushCbs(seen?: CountMap, i = flushIndex) {
+  if (__DEV__) {
+    seen = seen || new Map()
+  }
+  for (; i < queue.length; i++) {
+    const cb = queue[i]
+    if (cb && cb.pre) {
+      if (__DEV__ && checkRecursiveUpdates(seen!, cb)) {
         continue
       }
-      activePreFlushCbs[preFlushIndex]()
+      queue.splice(i, 1)
+      i--
+      cb()
     }
-    activePreFlushCbs = null
-    preFlushIndex = 0
-    currentPreFlushParentJob = null
-    // recursively flush until it drains
-    flushPreFlushCbs(seen, parentJob)
   }
 }
 
 export function flushPostFlushCbs(seen?: CountMap) {
-  // flush any pre cbs queued during the flush (e.g. pre watchers)
-  flushPreFlushCbs()
   if (pendingPostFlushCbs.length) {
     const deduped = [...new Set(pendingPostFlushCbs)]
     pendingPostFlushCbs.length = 0
@@ -222,6 +189,15 @@ export function flushPostFlushCbs(seen?: CountMap) {
 const getId = (job: SchedulerJob): number =>
   job.id == null ? Infinity : job.id
 
+const comparator = (a: SchedulerJob, b: SchedulerJob): number => {
+  const diff = getId(a) - getId(b)
+  if (diff === 0) {
+    if (a.pre && !b.pre) return -1
+    if (b.pre && !a.pre) return 1
+  }
+  return diff
+}
+
 function flushJobs(seen?: CountMap) {
   isFlushPending = false
   isFlushing = true
@@ -229,8 +205,6 @@ function flushJobs(seen?: CountMap) {
     seen = seen || new Map()
   }
 
-  flushPreFlushCbs(seen)
-
   // Sort queue before flush.
   // This ensures that:
   // 1. Components are updated from parent to child. (because parent is always
@@ -238,7 +212,7 @@ function flushJobs(seen?: CountMap) {
   //    priority number)
   // 2. If a component is unmounted during a parent component's update,
   //    its update can be skipped.
-  queue.sort((a, b) => getId(a) - getId(b))
+  queue.sort(comparator)
 
   // conditional usage of checkRecursiveUpdate must be determined out of
   // try ... catch block since Rollup by default de-optimizes treeshaking
@@ -270,11 +244,7 @@ function flushJobs(seen?: CountMap) {
     currentFlushPromise = null
     // some postFlushCb queued jobs!
     // keep flushing until it drains.
-    if (
-      queue.length ||
-      pendingPreFlushCbs.length ||
-      pendingPostFlushCbs.length
-    ) {
+    if (queue.length || pendingPostFlushCbs.length) {
       flushJobs(seen)
     }
   }