From a95532495af6a192981c89139c99f0621df05184 Mon Sep 17 00:00:00 2001 From: Evan You Date: Thu, 8 Nov 2018 20:08:28 -0500 Subject: [PATCH] refactor: properly cleanup invalidated jobs --- packages/runtime-core/src/componentUtils.ts | 5 +- packages/runtime-core/src/createRenderer.ts | 114 ++++++++++-------- .../scheduler/__tests__/scheduler.spec.ts | 33 +++-- packages/scheduler/src/experimental.ts | 48 ++++++-- packages/vue-compat/__tests__/compat.spec.ts | 9 +- 5 files changed, 130 insertions(+), 79 deletions(-) diff --git a/packages/runtime-core/src/componentUtils.ts b/packages/runtime-core/src/componentUtils.ts index 7115205fbd..48e457a6e8 100644 --- a/packages/runtime-core/src/componentUtils.ts +++ b/packages/runtime-core/src/componentUtils.ts @@ -25,6 +25,7 @@ import { } from './errorHandling' import { warn } from './warning' import { setCurrentInstance, unsetCurrentInstance } from './experimental/hooks' +import { stop } from '@vue/observer' let currentVNode: VNode | null = null let currentContextVNode: VNode | null = null @@ -150,9 +151,6 @@ export function renderFunctionalRoot(vnode: VNode): VNode { } export function teardownComponentInstance(instance: ComponentInstance) { - if (instance._unmounted) { - return - } const parentComponent = instance.$parent && instance.$parent._self if (parentComponent && !parentComponent._unmounted) { parentComponent.$children.splice( @@ -160,6 +158,7 @@ export function teardownComponentInstance(instance: ComponentInstance) { 1 ) } + stop(instance._updateHandle) teardownComputed(instance) teardownWatch(instance) } diff --git a/packages/runtime-core/src/createRenderer.ts b/packages/runtime-core/src/createRenderer.ts index 2aac3c6dbb..b1ecaafca1 100644 --- a/packages/runtime-core/src/createRenderer.ts +++ b/packages/runtime-core/src/createRenderer.ts @@ -5,7 +5,13 @@ import { immutable, AutorunOptions } from '@vue/observer' -import { queueJob, handleSchedulerError, nextTick } from '@vue/scheduler' +import { + queueJob, + handleSchedulerError, + nextTick, + queuePostCommitCb, + flushPostCommitCbs +} from '@vue/scheduler' import { VNodeFlags, ChildrenFlags } from './flags' import { EMPTY_OBJ, reservedPropRE, isString } from '@vue/shared' import { @@ -110,22 +116,6 @@ export function createRenderer(options: RendererOptions) { } } - // Lifecycle Hooks ----------------------------------------------------------- - - const lifecycleHooks: Function[] = [] - const vnodeUpdatedHooks: Function[] = [] - - function queuePostCommitHook(fn: Function) { - lifecycleHooks.push(fn) - } - - function flushHooks() { - let fn - while ((fn = lifecycleHooks.pop())) { - fn() - } - } - // mounting ------------------------------------------------------------------ function mount( @@ -197,12 +187,12 @@ export function createRenderer(options: RendererOptions) { insertOrAppend(container, el, endNode) } if (ref) { - queuePostCommitHook(() => { + queuePostCommitCb(() => { ref(el) }) } if (data != null && data.vnodeMounted) { - queuePostCommitHook(() => { + queuePostCommitCb(() => { data.vnodeMounted(vnode) }) } @@ -238,8 +228,17 @@ export function createRenderer(options: RendererOptions) { mountComponentInstance(vnode, container, isSVG, endNode) } else { queueJob(() => { - mountComponentInstance(vnode, container, isSVG, endNode) - }, flushHooks) + const instance = mountComponentInstance( + vnode, + container, + isSVG, + endNode + ) + // cleanup if mount is invalidated before committed + return () => { + teardownComponentInstance(instance) + } + }) } } } @@ -279,7 +278,7 @@ export function createRenderer(options: RendererOptions) { const subTree = (handle.prevTree = vnode.children = renderFunctionalRoot( vnode )) - queuePostCommitHook(() => { + queuePostCommitCb(() => { vnode.el = subTree.el as RenderNode }) mount(subTree, container, vnode as MountedVNode, isSVG, endNode) @@ -300,7 +299,13 @@ export function createRenderer(options: RendererOptions) { if (__COMPAT__) { doMount() } else { - queueJob(doMount) + queueJob(() => { + doMount() + // cleanup if mount is invalidated before committed + return () => { + stop(handle.runner) + } + }) } } @@ -313,7 +318,7 @@ export function createRenderer(options: RendererOptions) { const nextTree = (handle.prevTree = current.children = renderFunctionalRoot( current )) - queuePostCommitHook(() => { + queuePostCommitCb(() => { current.el = nextTree.el }) patch( @@ -349,7 +354,7 @@ export function createRenderer(options: RendererOptions) { const { children, childFlags } = vnode switch (childFlags) { case ChildrenFlags.SINGLE_VNODE: - queuePostCommitHook(() => { + queuePostCommitCb(() => { vnode.el = (children as MountedVNode).el }) mount(children as VNode, container, contextVNode, isSVG, endNode) @@ -360,7 +365,7 @@ export function createRenderer(options: RendererOptions) { vnode.el = placeholder.el break default: - queuePostCommitHook(() => { + queuePostCommitCb(() => { vnode.el = (children as MountedVNode[])[0].el }) mountArrayChildren( @@ -397,7 +402,7 @@ export function createRenderer(options: RendererOptions) { ) } if (ref) { - queuePostCommitHook(() => { + queuePostCommitCb(() => { ref(target) }) } @@ -529,9 +534,10 @@ export function createRenderer(options: RendererOptions) { ) if (nextData != null && nextData.vnodeUpdated) { - vnodeUpdatedHooks.push(() => { - nextData.vnodeUpdated(nextVNode, prevVNode) - }) + // TODO fix me + // vnodeUpdatedHooks.push(() => { + // nextData.vnodeUpdated(nextVNode, prevVNode) + // }) } } @@ -611,7 +617,7 @@ export function createRenderer(options: RendererOptions) { // then retrieve its next sibling to use as the end node for patchChildren. const endNode = platformNextSibling(getVNodeLastEl(prevVNode)) const { childFlags, children } = nextVNode - queuePostCommitHook(() => { + queuePostCommitCb(() => { switch (childFlags) { case ChildrenFlags.SINGLE_VNODE: nextVNode.el = (children as MountedVNode).el @@ -1192,7 +1198,7 @@ export function createRenderer(options: RendererOptions) { container: RenderNode | null, isSVG: boolean, endNode: RenderNode | null - ) { + ): ComponentInstance { if (__DEV__) { pushWarningContext(vnode) } @@ -1212,12 +1218,8 @@ export function createRenderer(options: RendererOptions) { $options: { beforeMount, renderTracked, renderTriggered } } = instance - if (beforeMount) { - callLifecycleHookWithHandler(beforeMount, $proxy, ErrorTypes.BEFORE_MOUNT) - } - const queueUpdate = (instance.$forceUpdate = () => { - queueJob(instance._updateHandle, flushHooks) + queueJob(instance._updateHandle) }) const autorunOptions: AutorunOptions = { @@ -1254,10 +1256,18 @@ export function createRenderer(options: RendererOptions) { if (instance._mounted) { updateComponentInstance(instance, isSVG) } else { + if (beforeMount) { + callLifecycleHookWithHandler( + beforeMount, + $proxy, + ErrorTypes.BEFORE_MOUNT + ) + } + // this will be executed synchronously right here instance.$vnode = renderInstanceRoot(instance) as MountedVNode - queuePostCommitHook(() => { + queuePostCommitCb(() => { vnode.el = instance.$vnode.el if (__COMPAT__) { // expose __vue__ for devtools @@ -1283,6 +1293,8 @@ export function createRenderer(options: RendererOptions) { if (__DEV__) { popWarningContext() } + + return instance } function updateComponentInstance( @@ -1312,7 +1324,7 @@ export function createRenderer(options: RendererOptions) { instance ) as MountedVNode) - queuePostCommitHook(() => { + queuePostCommitCb(() => { const el = nextVNode.el as RenderNode if (__COMPAT__) { // expose __vue__ for devtools @@ -1337,15 +1349,15 @@ export function createRenderer(options: RendererOptions) { nextVNode ) } - if (vnodeUpdatedHooks.length > 0) { - const vnodeUpdatedHooksForCurrentInstance = vnodeUpdatedHooks.slice() - vnodeUpdatedHooks.length = 0 - queuePostCommitHook(() => { - for (let i = 0; i < vnodeUpdatedHooksForCurrentInstance.length; i++) { - vnodeUpdatedHooksForCurrentInstance[i]() - } - }) - } + + // TODO fix me + // if (vnodeUpdatedHooks.length > 0) { + // const vnodeUpdatedHooksForCurrentInstance = vnodeUpdatedHooks.slice() + // vnodeUpdatedHooks.length = 0 + // for (let i = 0; i < vnodeUpdatedHooksForCurrentInstance.length; i++) { + // vnodeUpdatedHooksForCurrentInstance[i]() + // } + // } }) const container = platformParentNode(prevVNode.el) as RenderNode @@ -1363,7 +1375,6 @@ export function createRenderer(options: RendererOptions) { const { $vnode, $proxy, - _updateHandle, $options: { beforeUnmount, unmounted } } = instance if (beforeUnmount) { @@ -1376,7 +1387,6 @@ export function createRenderer(options: RendererOptions) { if ($vnode) { unmount($vnode) } - stop(_updateHandle) teardownComponentInstance(instance) instance._unmounted = true if (unmounted) { @@ -1402,7 +1412,7 @@ export function createRenderer(options: RendererOptions) { if (__DEV__) { popWarningContext() } - queuePostCommitHook(() => { + queuePostCommitCb(() => { callActivatedHook(instance, true) }) } @@ -1486,7 +1496,7 @@ export function createRenderer(options: RendererOptions) { } } if (__COMPAT__) { - flushHooks() + flushPostCommitCbs() return vnode && vnode.flags & VNodeFlags.COMPONENT_STATEFUL ? (vnode.children as ComponentInstance).$proxy : null diff --git a/packages/scheduler/__tests__/scheduler.spec.ts b/packages/scheduler/__tests__/scheduler.spec.ts index 74c21daeca..159e0e6148 100644 --- a/packages/scheduler/__tests__/scheduler.spec.ts +++ b/packages/scheduler/__tests__/scheduler.spec.ts @@ -1,4 +1,4 @@ -import { queueJob, nextTick } from '../src/index' +import { queueJob, queuePostCommitCb, nextTick } from '../src/index' describe('scheduler', () => { it('queueJob', async () => { @@ -32,13 +32,15 @@ describe('scheduler', () => { expect(calls).toEqual(['job1', 'job2']) }) - it('queueJob w/ postFlushCb', async () => { + it('queueJob w/ postCommitCb', async () => { const calls: any = [] const job1 = () => { calls.push('job1') + queuePostCommitCb(cb1) } const job2 = () => { calls.push('job2') + queuePostCommitCb(cb2) } const cb1 = () => { calls.push('cb1') @@ -46,21 +48,24 @@ describe('scheduler', () => { const cb2 = () => { calls.push('cb2') } - queueJob(job1, cb1) - queueJob(job2, cb2) + queueJob(job1) + queueJob(job2) await nextTick() - expect(calls).toEqual(['job1', 'job2', 'cb1', 'cb2']) + // post commit cbs are called in reverse! + expect(calls).toEqual(['job1', 'job2', 'cb2', 'cb1']) }) it('queueJob w/ postFlushCb while flushing', async () => { const calls: any = [] const job1 = () => { calls.push('job1') + queuePostCommitCb(cb1) // job1 queues job2 - queueJob(job2, cb2) + queueJob(job2) } const job2 = () => { calls.push('job2') + queuePostCommitCb(cb2) } const cb1 = () => { calls.push('cb1') @@ -68,10 +73,10 @@ describe('scheduler', () => { const cb2 = () => { calls.push('cb2') } - queueJob(job1, cb1) + queueJob(job1) expect(calls).toEqual([]) await nextTick() - expect(calls).toEqual(['job1', 'job2', 'cb1', 'cb2']) + expect(calls).toEqual(['job1', 'job2', 'cb2', 'cb1']) }) it('should dedupe queued tasks', async () => { @@ -91,26 +96,28 @@ describe('scheduler', () => { expect(calls).toEqual(['job1', 'job2']) }) - it('queueJob inside postFlushCb', async () => { + it('queueJob inside postCommitCb', async () => { const calls: any = [] const job1 = () => { calls.push('job1') + queuePostCommitCb(cb1) } const cb1 = () => { // queue another job in postFlushCb calls.push('cb1') - queueJob(job2, cb2) + queueJob(job2) } const job2 = () => { calls.push('job2') + queuePostCommitCb(cb2) } const cb2 = () => { calls.push('cb2') } - queueJob(job1, cb1) - queueJob(job2, cb2) + queueJob(job1) + queueJob(job2) await nextTick() - expect(calls).toEqual(['job1', 'job2', 'cb1', 'cb2', 'job2', 'cb2']) + expect(calls).toEqual(['job1', 'job2', 'cb2', 'cb1', 'job2', 'cb2']) }) }) diff --git a/packages/scheduler/src/experimental.ts b/packages/scheduler/src/experimental.ts index 9b2380b32e..8697211190 100644 --- a/packages/scheduler/src/experimental.ts +++ b/packages/scheduler/src/experimental.ts @@ -1,8 +1,11 @@ +// TODO infinite updates detection + import { Op, setCurrentOps } from './patchNodeOps' interface Job extends Function { ops: Op[] - post: Function | null + post: Function[] + cleanup: Function | null expiration: number } @@ -12,6 +15,8 @@ const enum Priorities { type ErrorHandler = (err: Error) => any +let currentJob: Job | null = null + let start: number = 0 const getNow = () => window.performance.now() const frameBudget = __JSDOM__ ? Infinity : 1000 / 60 @@ -85,10 +90,10 @@ export function handleSchedulerError(handler: ErrorHandler) { let hasPendingFlush = false -export function queueJob(rawJob: Function, postJob?: Function | null) { +export function queueJob(rawJob: Function) { const job = rawJob as Job - job.post = postJob || null job.ops = job.ops || [] + job.post = job.post || [] // 1. let's see if this invalidates any work that // has already been done. const commitIndex = commitQueue.indexOf(job) @@ -118,6 +123,24 @@ export function queueJob(rawJob: Function, postJob?: Function | null) { } } +export function queuePostCommitCb(fn: Function) { + if (currentJob) { + currentJob.post.push(fn) + } else { + postCommitQueue.push(fn) + } +} + +export function flushPostCommitCbs() { + // post commit hooks (updated, mounted) + // this queue is flushed in reverse becuase these hooks should be invoked + // child first + let job + while ((job = postCommitQueue.pop())) { + job() + } +} + function flush(): void { let job while (true) { @@ -139,14 +162,12 @@ function flush(): void { // all done, time to commit! while ((job = commitQueue.shift())) { commitJob(job) - if (job.post && postCommitQueue.indexOf(job.post) < 0) { - postCommitQueue.push(job.post) + if (job.post) { + postCommitQueue.push(...job.post) + job.post.length = 0 } } - // post commit hooks (updated, mounted) - while ((job = postCommitQueue.shift())) { - job() - } + flushPostCommitCbs() // some post commit hook triggered more updates... if (patchQueue.length > 0) { if (!__COMPAT__ && getNow() - start > frameBudget) { @@ -174,7 +195,9 @@ function patchJob(job: Job) { // job with existing ops means it's already been patched in a low priority queue if (job.ops.length === 0) { setCurrentOps(job.ops) - job() + currentJob = job + job.cleanup = job() + currentJob = null setCurrentOps(null) commitQueue.push(job) } @@ -190,4 +213,9 @@ function commitJob({ ops }: Job) { function invalidateJob(job: Job) { job.ops.length = 0 + job.post.length = 0 + if (job.cleanup) { + job.cleanup() + job.cleanup = null + } } diff --git a/packages/vue-compat/__tests__/compat.spec.ts b/packages/vue-compat/__tests__/compat.spec.ts index b1aa63c26c..38b67f7664 100644 --- a/packages/vue-compat/__tests__/compat.spec.ts +++ b/packages/vue-compat/__tests__/compat.spec.ts @@ -7,6 +7,9 @@ describe('2.x compat build', async () => { const root = document.createElement('div') document.body.appendChild(root) + const mounted = jest.fn() + const updated = jest.fn() + const instance = new Vue({ data() { return { count: 0 } @@ -18,15 +21,19 @@ describe('2.x compat build', async () => { }, render(h: any) { return h('div', this.count) - } + }, + mounted, + updated }).$mount(root) expect(instance.count).toBe(0) expect(root.textContent).toBe('0') + expect(mounted).toHaveBeenCalled() instance.change() expect(instance.count).toBe(1) await Vue.nextTick() expect(root.textContent).toBe('1') + expect(updated).toHaveBeenCalled() }) }) -- 2.47.3