From caccec3f78414ae294f1a813ffd16791d56da3a6 Mon Sep 17 00:00:00 2001 From: HcySunYang Date: Fri, 14 Aug 2020 21:50:23 +0800 Subject: [PATCH] fix(runtime-core/scheduler): sort postFlushCbs to ensure refs are set before lifecycle hooks (#1854) fix #1852 --- .../runtime-core/__tests__/apiWatch.spec.ts | 33 ++++++++++++++++++- .../runtime-core/__tests__/scheduler.spec.ts | 16 +++++++++ packages/runtime-core/src/renderer.ts | 14 +++++--- packages/runtime-core/src/scheduler.ts | 31 ++++++++++------- 4 files changed, 76 insertions(+), 18 deletions(-) diff --git a/packages/runtime-core/__tests__/apiWatch.spec.ts b/packages/runtime-core/__tests__/apiWatch.spec.ts index 97dd131e812..edeedf6c2af 100644 --- a/packages/runtime-core/__tests__/apiWatch.spec.ts +++ b/packages/runtime-core/__tests__/apiWatch.spec.ts @@ -7,7 +7,7 @@ import { ref, h } from '../src/index' -import { render, nodeOps, serializeInner } from '@vue/runtime-test' +import { render, nodeOps, serializeInner, TestElement } from '@vue/runtime-test' import { ITERATE_KEY, DebuggerEvent, @@ -484,6 +484,37 @@ describe('api: watch', () => { expect(calls).toEqual(['render', 'watcher 1', 'watcher 2', 'render']) }) + // #1852 + it('flush: post watcher should fire after template refs updated', async () => { + const toggle = ref(false) + let dom: TestElement | null = null + + const App = { + setup() { + const domRef = ref(null) + + watch( + toggle, + () => { + dom = domRef.value + }, + { flush: 'post' } + ) + + return () => { + return toggle.value ? h('p', { ref: domRef }) : null + } + } + } + + render(h(App), nodeOps.createElement('div')) + expect(dom).toBe(null) + + toggle.value = true + await nextTick() + expect(dom!.tag).toBe('p') + }) + it('deep', async () => { const state = reactive({ nested: { diff --git a/packages/runtime-core/__tests__/scheduler.spec.ts b/packages/runtime-core/__tests__/scheduler.spec.ts index 1811ce6b390..53b0890fd7c 100644 --- a/packages/runtime-core/__tests__/scheduler.spec.ts +++ b/packages/runtime-core/__tests__/scheduler.spec.ts @@ -403,6 +403,22 @@ describe('scheduler', () => { expect(calls).toEqual(['job3', 'job2', 'job1']) }) + test('sort SchedulerCbs based on id', async () => { + const calls: string[] = [] + const cb1 = () => calls.push('cb1') + // cb1 has no id + const cb2 = () => calls.push('cb2') + cb2.id = 2 + const cb3 = () => calls.push('cb3') + cb3.id = 1 + + queuePostFlushCb(cb1) + queuePostFlushCb(cb2) + queuePostFlushCb(cb3) + await nextTick() + expect(calls).toEqual(['cb3', 'cb2', 'cb1']) + }) + // #1595 test('avoid duplicate postFlushCb invocation', async () => { const calls: string[] = [] diff --git a/packages/runtime-core/src/renderer.ts b/packages/runtime-core/src/renderer.ts index cb1e3869345..4d224e40573 100644 --- a/packages/runtime-core/src/renderer.ts +++ b/packages/runtime-core/src/renderer.ts @@ -42,7 +42,8 @@ import { flushPostFlushCbs, invalidateJob, flushPreFlushCbs, - SchedulerJob + SchedulerJob, + SchedulerCb } from './scheduler' import { effect, stop, ReactiveEffectOptions, isRef } from '@vue/reactivity' import { updateProps } from './componentProps' @@ -330,17 +331,20 @@ export const setRef = ( // null values means this is unmount and it should not overwrite another // ref with the same key if (value) { + ;(doSet as SchedulerCb).id = -1 queuePostRenderEffect(doSet, parentSuspense) } else { doSet() } } else if (isRef(ref)) { + const doSet = () => { + ref.value = value + } if (value) { - queuePostRenderEffect(() => { - ref.value = value - }, parentSuspense) + ;(doSet as SchedulerCb).id = -1 + queuePostRenderEffect(doSet, parentSuspense) } else { - ref.value = value + doSet() } } else if (isFunction(ref)) { callWithErrorHandling(ref, parentComponent, ErrorCodes.FUNCTION_REF, [ diff --git a/packages/runtime-core/src/scheduler.ts b/packages/runtime-core/src/scheduler.ts index 1f168b7363e..ab8a0f347d2 100644 --- a/packages/runtime-core/src/scheduler.ts +++ b/packages/runtime-core/src/scheduler.ts @@ -22,18 +22,21 @@ export interface SchedulerJob { allowRecurse?: boolean } +export type SchedulerCb = Function & { id?: number } +export type SchedulerCbs = SchedulerCb | SchedulerCb[] + let isFlushing = false let isFlushPending = false const queue: (SchedulerJob | null)[] = [] let flushIndex = 0 -const pendingPreFlushCbs: Function[] = [] -let activePreFlushCbs: Function[] | null = null +const pendingPreFlushCbs: SchedulerCb[] = [] +let activePreFlushCbs: SchedulerCb[] | null = null let preFlushIndex = 0 -const pendingPostFlushCbs: Function[] = [] -let activePostFlushCbs: Function[] | null = null +const pendingPostFlushCbs: SchedulerCb[] = [] +let activePostFlushCbs: SchedulerCb[] | null = null let postFlushIndex = 0 const resolvedPromise: Promise = Promise.resolve() @@ -42,7 +45,7 @@ let currentFlushPromise: Promise | null = null let currentPreFlushParentJob: SchedulerJob | null = null const RECURSION_LIMIT = 100 -type CountMap = Map +type CountMap = Map export function nextTick(fn?: () => void): Promise { const p = currentFlushPromise || resolvedPromise @@ -84,9 +87,9 @@ export function invalidateJob(job: SchedulerJob) { } function queueCb( - cb: Function | Function[], - activeQueue: Function[] | null, - pendingQueue: Function[], + cb: SchedulerCbs, + activeQueue: SchedulerCb[] | null, + pendingQueue: SchedulerCb[], index: number ) { if (!isArray(cb)) { @@ -108,11 +111,11 @@ function queueCb( queueFlush() } -export function queuePreFlushCb(cb: Function) { +export function queuePreFlushCb(cb: SchedulerCb) { queueCb(cb, activePreFlushCbs, pendingPreFlushCbs, preFlushIndex) } -export function queuePostFlushCb(cb: Function | Function[]) { +export function queuePostFlushCb(cb: SchedulerCbs) { queueCb(cb, activePostFlushCbs, pendingPostFlushCbs, postFlushIndex) } @@ -152,6 +155,9 @@ export function flushPostFlushCbs(seen?: CountMap) { if (__DEV__) { seen = seen || new Map() } + + activePostFlushCbs.sort((a, b) => getId(a) - getId(b)) + for ( postFlushIndex = 0; postFlushIndex < activePostFlushCbs.length; @@ -167,7 +173,8 @@ export function flushPostFlushCbs(seen?: CountMap) { } } -const getId = (job: SchedulerJob) => (job.id == null ? Infinity : job.id) +const getId = (job: SchedulerJob | SchedulerCb) => + job.id == null ? Infinity : job.id function flushJobs(seen?: CountMap) { isFlushPending = false @@ -215,7 +222,7 @@ function flushJobs(seen?: CountMap) { } } -function checkRecursiveUpdates(seen: CountMap, fn: SchedulerJob | Function) { +function checkRecursiveUpdates(seen: CountMap, fn: SchedulerJob | SchedulerCb) { if (!seen.has(fn)) { seen.set(fn, 1) } else {