fix(ssr): memory leak in poll method (#2875)

Co-authored-by: Eduardo San Martin Morote <[email protected]> Co-authored-by: Eduardo San Martin Morote <[email protected]>
vuejs · Oct 6, 2020 · 7693eb5 · 7693eb5
1 parent f597959
commit 7693eb5
Show file tree

Hide file tree

Showing 7 changed files with 60 additions and 49 deletions.
diff --git a/examples/navigation-guards/app.js b/examples/navigation-guards/app.js
@@ -91,15 +91,25 @@ const Quux = {
 }
 
 const NestedParent = {
-  template: `<div id="nested-parent">Nested Parent <hr>
-  <router-link to="/parent/child/1">/parent/child/1</router-link>
-  <router-link to="/parent/child/2">/parent/child/2</router-link>
-  <hr>
-  <p id="bre-order">
-    <span v-for="log in logs">{{ log }} </span>
-  </p>
-
-  <router-view/></div>`,
+  template: `
+  <div id="nested-parent">
+    Nested Parent
+    <hr>
+    <router-link to="/parent/child/1">/parent/child/1</router-link>
+    <router-link to="/parent/child/2">/parent/child/2</router-link>
+    <hr>
+    <p id="bre-order">
+      <span v-for="log in logs">{{ log }} </span>
+    </p>
+    <!-- #705 -->
+    <!-- Some transitions, specifically out-in transitions, cause the view -->
+    <!-- that is transitioning in to appear significantly later than normal, -->
+    <!-- which can cause bugs as "vm" below in "next(vm => ...)" may not be -->
+    <!-- available at the time the callback is called -->
+    <transition name="fade" mode="out-in">
+      <router-view :key="$route.path"/>
+    </transition>
+  </div>`,
   data: () => ({ logs: [] }),
   beforeRouteEnter (to, from, next) {
     next(vm => {

diff --git a/examples/navigation-guards/index.html b/examples/navigation-guards/index.html
@@ -1,5 +1,13 @@
 <!DOCTYPE html>
 <link rel="stylesheet" href="/global.css">
+<style>
+.fade-enter-active, .fade-leave-active {
+  transition: opacity .5s ease;
+}
+.fade-enter, .fade-leave-active {
+  opacity: 0;
+}
+</style>
 <a href="/">&larr; Examples index</a>
 <div id="app"></div>
 <script src="/__build__/shared.chunk.js"></script>

diff --git a/flow/declarations.js b/flow/declarations.js
@@ -68,6 +68,7 @@ declare type RouteRecord = {
   regex: RouteRegExp;
   components: Dictionary<any>;
   instances: Dictionary<any>;
+  enteredCbs: Dictionary<Array<Function>>;
   name: ?string;
   parent: ?RouteRecord;
   redirect: ?RedirectOption;

diff --git a/src/components/view.js b/src/components/view.js
@@ -1,5 +1,6 @@
 import { warn } from '../util/warn'
 import { extend } from '../util/misc'
+import { handleRouteEntered } from '../util/route'
 
 export default {
   name: 'RouterView',
@@ -94,6 +95,11 @@ export default {
       ) {
         matched.instances[name] = vnode.componentInstance
       }
+
+      // if the route transition has already been confirmed then we weren't
+      // able to call the cbs during confirmation as the component was not
+      // registered yet, so we call it here.
+      handleRouteEntered(route)
     }
 
     const configProps = matched.props && matched.props[name]

diff --git a/src/create-route-map.js b/src/create-route-map.js
@@ -85,6 +85,7 @@ function addRouteRecord (
     regex: compileRouteRegex(normalizedPath, pathToRegexpOptions),
     components: route.components || { default: route.component },
     instances: {},
+    enteredCbs: {},
     name,
     parent,
     matchAs,

diff --git a/src/history/base.js b/src/history/base.js
@@ -5,7 +5,7 @@ import type Router from '../index'
 import { inBrowser } from '../util/dom'
 import { runQueue } from '../util/async'
 import { warn } from '../util/warn'
-import { START, isSameRoute } from '../util/route'
+import { START, isSameRoute, handleRouteEntered } from '../util/route'
 import {
   flatten,
   flatMapComponents,
@@ -218,11 +218,9 @@ export class History {
     }
 
     runQueue(queue, iterator, () => {
-      const postEnterCbs = []
-      const isValid = () => this.current === route
       // wait until async components are resolved before
       // extracting in-component enter guards
-      const enterGuards = extractEnterGuards(activated, postEnterCbs, isValid)
+      const enterGuards = extractEnterGuards(activated)
       const queue = enterGuards.concat(this.router.resolveHooks)
       runQueue(queue, iterator, () => {
         if (this.pending !== route) {
@@ -232,9 +230,7 @@ export class History {
         onComplete(route)
         if (this.router.app) {
           this.router.app.$nextTick(() => {
-            postEnterCbs.forEach(cb => {
-              cb()
-            })
+            handleRouteEntered(route)
           })
         }
       })
@@ -352,57 +348,31 @@ function bindGuard (guard: NavigationGuard, instance: ?_Vue): ?NavigationGuard {
 }
 
 function extractEnterGuards (
-  activated: Array<RouteRecord>,
-  cbs: Array<Function>,
-  isValid: () => boolean
+  activated: Array<RouteRecord>
 ): Array<?Function> {
   return extractGuards(
     activated,
     'beforeRouteEnter',
     (guard, _, match, key) => {
-      return bindEnterGuard(guard, match, key, cbs, isValid)
+      return bindEnterGuard(guard, match, key)
     }
   )
 }
 
 function bindEnterGuard (
   guard: NavigationGuard,
   match: RouteRecord,
-  key: string,
-  cbs: Array<Function>,
-  isValid: () => boolean
+  key: string
 ): NavigationGuard {
   return function routeEnterGuard (to, from, next) {
     return guard(to, from, cb => {
       if (typeof cb === 'function') {
-        cbs.push(() => {
-          // #750
-          // if a router-view is wrapped with an out-in transition,
-          // the instance may not have been registered at this time.
-          // we will need to poll for registration until current route
-          // is no longer valid.
-          poll(cb, match.instances, key, isValid)
-        })
+        if (!match.enteredCbs[key]) {
+          match.enteredCbs[key] = []
+        }
+        match.enteredCbs[key].push(cb)
       }
       next(cb)
     })
   }
 }
-
-function poll (
-  cb: any, // somehow flow cannot infer this is a function
-  instances: Object,
-  key: string,
-  isValid: () => boolean
-) {
-  if (
-    instances[key] &&
-    !instances[key]._isBeingDestroyed // do not reuse being destroyed instance
-  ) {
-    cb(instances[key])
-  } else if (isValid()) {
-    setTimeout(() => {
-      poll(cb, instances, key, isValid)
-    }, 16)
-  }
-}
diff --git a/src/util/route.js b/src/util/route.js
@@ -132,3 +132,18 @@ function queryIncludes (current: Dictionary<string>, target: Dictionary<string>)
   }
   return true
 }
+
+export function handleRouteEntered (route: Route) {
+  for (let i = 0; i < route.matched.length; i++) {
+    const record = route.matched[i]
+    for (const name in record.instances) {
+      const instance = record.instances[name]
+      const cbs = record.enteredCbs[name]
+      if (!instance || !cbs) continue
+      delete record.enteredCbs[name]
+      for (let i = 0; i < cbs.length; i++) {
+        if (!instance._isBeingDestroyed) cbs[i](instance)
+      }
+    }
+  }
+}