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(ssr): memory leak in poll method #2875

Merged
merged 12 commits into from
Oct 6, 2020
28 changes: 19 additions & 9 deletions examples/navigation-guards/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 -->
posva marked this conversation as resolved.
Show resolved Hide resolved
<!-- 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 => {
Expand Down
8 changes: 8 additions & 0 deletions examples/navigation-guards/index.html
Original file line number Diff line number Diff line change
@@ -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>
Expand Down
1 change: 1 addition & 0 deletions flow/declarations.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 6 additions & 0 deletions src/components/view.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { warn } from '../util/warn'
import { extend } from '../util/misc'
import { handleRouteEntered } from '../util/route'

export default {
name: 'RouterView',
Expand Down Expand Up @@ -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]
Expand Down
1 change: 1 addition & 0 deletions src/create-route-map.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ function addRouteRecord (
regex: compileRouteRegex(normalizedPath, pathToRegexpOptions),
components: route.components || { default: route.component },
instances: {},
enteredCbs: {},
name,
parent,
matchAs,
Expand Down
50 changes: 10 additions & 40 deletions src/history/base.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -220,11 +220,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) {
Expand All @@ -234,9 +232,7 @@ export class History {
onComplete(route)
if (this.router.app) {
this.router.app.$nextTick(() => {
postEnterCbs.forEach(cb => {
cb()
})
handleRouteEntered(route)
})
}
})
Expand Down Expand Up @@ -347,57 +343,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)
}
}
15 changes: 15 additions & 0 deletions src/util/route.js
Original file line number Diff line number Diff line change
Expand Up @@ -130,3 +130,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)
}
}
}
}