]> git.ipfire.org Git - thirdparty/vuejs/core.git/commitdiff
refactor: properly cleanup invalidated jobs
authorEvan You <yyx990803@gmail.com>
Fri, 9 Nov 2018 01:08:28 +0000 (20:08 -0500)
committerEvan You <yyx990803@gmail.com>
Fri, 9 Nov 2018 01:08:28 +0000 (20:08 -0500)
packages/runtime-core/src/componentUtils.ts
packages/runtime-core/src/createRenderer.ts
packages/scheduler/__tests__/scheduler.spec.ts
packages/scheduler/src/experimental.ts
packages/vue-compat/__tests__/compat.spec.ts

index 7115205fbd3b2115848ede82ca273437c9344f2d..48e457a6e82473eec282ff2857e7d54bae21e73d 100644 (file)
@@ -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)
 }
index 2aac3c6dbb6fba0b03d600ac71056097bec333e0..b1ecaafca16d659f1bb246038b58ed61186f0543 100644 (file)
@@ -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
index 74c21daeca0f6d01865d47a789de91a0cfceacaf..159e0e614878d0bcc02c0925dd295814da7ad19c 100644 (file)
@@ -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'])
   })
 })
index 9b2380b32ecca6eb52d5900dd24340f0d7fbe3ac..86972111905d9c89ff4726199e0a9d3756c91e38 100644 (file)
@@ -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
+  }
 }
index b1aa63c26ce5fecc46d251d422a38333e1ca7327..38b67f7664fc9af501d8364e756943364471af79 100644 (file)
@@ -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()
   })
 })