fix(scheduler): sort jobs before flushing

This fixes the case where a child component is added to the queue before
its parent, but should be invalidated by its parent's update. Same logic
was present in Vue 2.

Properly fixes #910
ref: https://github.com/vuejs/vue-next/issues/910#issuecomment-613097539

diff --git a/packages/reactivity/src/effect.ts b/packages/reactivity/src/effect.ts
index 6a09640e88fb8b9576d5e815ece2eefaa0b448ed..aab4ae6db6ab7f0790674520b32aea6d4110ca9e 100644 (file)
--- a/packages/reactivity/src/effect.ts
+++ b/packages/reactivity/src/effect.ts
@@ -12,6 +12,7 @@ const targetMap = new WeakMap<any, KeyToDepMap>()
 export interface ReactiveEffect<T = any> {
   (...args: any[]): T
   _isEffect: true
+  id: number
   active: boolean
   raw: () => T
   deps: Array<Dep>
@@ -21,7 +22,7 @@ export interface ReactiveEffect<T = any> {
 export interface ReactiveEffectOptions {
   lazy?: boolean
   computed?: boolean
-  scheduler?: (job: () => void) => void
+  scheduler?: (job: ReactiveEffect) => void
   onTrack?: (event: DebuggerEvent) => void
   onTrigger?: (event: DebuggerEvent) => void
   onStop?: () => void
@@ -74,6 +75,8 @@ export function stop(effect: ReactiveEffect) {
   }
 }
 
+let uid = 0
+
 function createReactiveEffect<T = any>(
   fn: (...args: any[]) => T,
   options: ReactiveEffectOptions
@@ -96,6 +99,7 @@ function createReactiveEffect<T = any>(
       }
     }
   } as ReactiveEffect
+  effect.id = uid++
   effect._isEffect = true
   effect.active = true
   effect.raw = fn

diff --git a/packages/runtime-core/__tests__/scheduler.spec.ts b/packages/runtime-core/__tests__/scheduler.spec.ts
index cf035c97a0d9dd7f23783ebbe0623107edb16124..a4554325aaaa03428ff1828f80258725154a76a9 100644 (file)
--- a/packages/runtime-core/__tests__/scheduler.spec.ts
+++ b/packages/runtime-core/__tests__/scheduler.spec.ts
@@ -262,4 +262,20 @@ describe('scheduler', () => {
     // job2 should be called only once
     expect(calls).toEqual(['job1', 'job2', 'job3', 'job4'])
   })
+
+  test('sort job based on id', async () => {
+    const calls: string[] = []
+    const job1 = () => calls.push('job1')
+    // job1 has no id
+    const job2 = () => calls.push('job2')
+    job2.id = 2
+    const job3 = () => calls.push('job3')
+    job3.id = 1
+
+    queueJob(job1)
+    queueJob(job2)
+    queueJob(job3)
+    await nextTick()
+    expect(calls).toEqual(['job3', 'job2', 'job1'])
+  })
 })

diff --git a/packages/runtime-core/src/scheduler.ts b/packages/runtime-core/src/scheduler.ts
index c730730f416550a2578012231a494d25ff514144..e518427d27095d1253351e5ec881818ee244db37 100644 (file)
--- a/packages/runtime-core/src/scheduler.ts
+++ b/packages/runtime-core/src/scheduler.ts
@@ -1,7 +1,12 @@
 import { ErrorCodes, callWithErrorHandling } from './errorHandling'
 import { isArray } from '@vue/shared'
 
-const queue: (Function | null)[] = []
+export interface Job {
+  (): void
+  id?: number
+}
+
+const queue: (Job | null)[] = []
 const postFlushCbs: Function[] = []
 const p = Promise.resolve()
 
@@ -9,20 +14,20 @@ let isFlushing = false
 let isFlushPending = false
 
 const RECURSION_LIMIT = 100
-type CountMap = Map<Function, number>
+type CountMap = Map<Job | Function, number>
 
 export function nextTick(fn?: () => void): Promise<void> {
   return fn ? p.then(fn) : p
 }
 
-export function queueJob(job: () => void) {
+export function queueJob(job: Job) {
   if (!queue.includes(job)) {
     queue.push(job)
     queueFlush()
   }
 }
 
-export function invalidateJob(job: () => void) {
+export function invalidateJob(job: Job) {
   const i = queue.indexOf(job)
   if (i > -1) {
     queue[i] = null
@@ -45,11 +50,9 @@ function queueFlush() {
   }
 }
 
-const dedupe = (cbs: Function[]): Function[] => [...new Set(cbs)]
-
 export function flushPostFlushCbs(seen?: CountMap) {
   if (postFlushCbs.length) {
-    const cbs = dedupe(postFlushCbs)
+    const cbs = [...new Set(postFlushCbs)]
     postFlushCbs.length = 0
     if (__DEV__) {
       seen = seen || new Map()
@@ -63,6 +66,8 @@ export function flushPostFlushCbs(seen?: CountMap) {
   }
 }
 
+const getId = (job: Job) => (job.id == null ? Infinity : job.id)
+
 function flushJobs(seen?: CountMap) {
   isFlushPending = false
   isFlushing = true
@@ -70,6 +75,18 @@ function flushJobs(seen?: CountMap) {
   if (__DEV__) {
     seen = seen || new Map()
   }
+
+  // Sort queue before flush.
+  // This ensures that:
+  // 1. Components are updated from parent to child. (because parent is always
+  //    created before the child so its render effect will have smaller
+  //    priority number)
+  // 2. If a component is unmounted during a parent component's update,
+  //    its update can be skipped.
+  // Jobs can never be null before flush starts, since they are only invalidated
+  // during execution of another flushed job.
+  queue.sort((a, b) => getId(a!) - getId(b!))
+
   while ((job = queue.shift()) !== undefined) {
     if (job === null) {
       continue
@@ -88,7 +105,7 @@ function flushJobs(seen?: CountMap) {
   }
 }
 
-function checkRecursiveUpdates(seen: CountMap, fn: Function) {
+function checkRecursiveUpdates(seen: CountMap, fn: Job | Function) {
   if (!seen.has(fn)) {
     seen.set(fn, 1)
   } else {