Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(scheduler): allows to sort the PostFlushCbs #1854

Merged
merged 1 commit into from
Aug 14, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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