Skip to content

Commit

Permalink
fix(watch): exhaust pre-flush watchers + avoid duplicate render by pr…
Browse files Browse the repository at this point in the history
…e-flush watchers

close #1777
  • Loading branch information
yyx990803 committed Aug 4, 2020
1 parent b5f91ff commit a0e34ce
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 7 deletions.
16 changes: 14 additions & 2 deletions packages/runtime-core/__tests__/apiWatch.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,7 @@ describe('api: watch', () => {
it('flush: pre watcher watching props should fire before child update', async () => {
const a = ref(0)
const b = ref(0)
const c = ref(0)
const calls: string[] = []

const Comp = {
Expand All @@ -444,11 +445,22 @@ describe('api: watch', () => {
watch(
() => props.a + props.b,
() => {
calls.push('watcher')
calls.push('watcher 1')
c.value++
},
{ flush: 'pre' }
)

// #1777 chained pre-watcher
watch(
c,
() => {
calls.push('watcher 2')
},
{ flush: 'pre' }
)
return () => {
c.value
calls.push('render')
}
}
Expand All @@ -469,7 +481,7 @@ describe('api: watch', () => {
a.value++
b.value++
await nextTick()
expect(calls).toEqual(['render', 'watcher', 'render'])
expect(calls).toEqual(['render', 'watcher 1', 'watcher 2', 'render'])
})

it('deep', async () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/runtime-core/src/renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1430,7 +1430,7 @@ function baseCreateRenderer(
instance.next = null
updateProps(instance, nextVNode.props, prevProps, optimized)
updateSlots(instance, nextVNode.children)
runPreflushJobs()
runPreflushJobs(instance.update)
}

const patchChildren: PatchChildrenFn = (
Expand Down
27 changes: 23 additions & 4 deletions packages/runtime-core/src/scheduler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ let flushIndex = 0
let pendingPostFlushCbs: Function[] | null = null
let pendingPostFlushIndex = 0
let hasPendingPreFlushJobs = false
let currentPreFlushParentJob: SchedulerJob | null = null

const RECURSION_LIMIT = 100
type CountMap = Map<SchedulerJob | Function, number>
Expand All @@ -44,9 +45,16 @@ 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.cb ? flushIndex + 1 : flushIndex)
(!queue.length ||
!queue.includes(
job,
isFlushing && job.cb ? flushIndex + 1 : flushIndex
)) &&
job !== currentPreFlushParentJob
) {
if (job.id && job.id > 0) {
debugger

This comment has been minimized.

Copy link
@Niputi

Niputi Aug 4, 2020

I think you accidentally forgot to delete the "debugger" here

}
queue.push(job)
if ((job.id as number) < 0) hasPendingPreFlushJobs = true
queueFlush()
Expand All @@ -60,16 +68,27 @@ export function invalidateJob(job: SchedulerJob) {
}
}

export function runPreflushJobs() {
/**
* Run flush: 'pre' watcher callbacks. This is only called in
* `updateComponentPreRender` to cover the case where pre-flush watchers are
* triggered by the change of a component's props. This means the scheduler is
* already flushing and we are already inside the component's update effect,
* right when the render function is about to be called. So if the watcher
* triggers the same component to update, we don't want it to be queued (this
* is checked via `currentPreFlushParentJob`).
*/
export function runPreflushJobs(parentJob: SchedulerJob) {
if (hasPendingPreFlushJobs) {
currentPreFlushParentJob = parentJob
hasPendingPreFlushJobs = false
for (let job, i = queue.length - 1; i > flushIndex; i--) {
for (let job, i = flushIndex + 1; i < queue.length; i++) {
job = queue[i]
if (job && (job.id as number) < 0) {
job()
queue[i] = null
}
}
currentPreFlushParentJob = null
}
}

Expand Down

0 comments on commit a0e34ce

Please sign in to comment.