Skip to content

Commit

Permalink
fix(runtime-core/scheduler): sort postFlushCbs to ensure refs are set…
Browse files Browse the repository at this point in the history
… before lifecycle hooks (#1854)

fix #1852
  • Loading branch information
HcySunYang authored Aug 14, 2020
1 parent 3fa5c9f commit caccec3
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 18 deletions.
33 changes: 32 additions & 1 deletion packages/runtime-core/__tests__/apiWatch.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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<TestElement | null>(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: {
Expand Down
16 changes: 16 additions & 0 deletions packages/runtime-core/__tests__/scheduler.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[] = []
Expand Down
14 changes: 9 additions & 5 deletions packages/runtime-core/src/renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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, [
Expand Down
31 changes: 19 additions & 12 deletions packages/runtime-core/src/scheduler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<any> = Promise.resolve()
Expand All @@ -42,7 +45,7 @@ let currentFlushPromise: Promise<void> | null = null
let currentPreFlushParentJob: SchedulerJob | null = null

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

export function nextTick(fn?: () => void): Promise<void> {
const p = currentFlushPromise || resolvedPromise
Expand Down Expand Up @@ -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)) {
Expand All @@ -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)
}

Expand Down Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit caccec3

Please sign in to comment.