Skip to content

Commit

Permalink
re-implement mergeVNodeHook to prevent memory leak (fix #4990)
Browse files Browse the repository at this point in the history
  • Loading branch information
yyx990803 committed Feb 23, 2017
1 parent 01b09e6 commit 2a5fb41
Show file tree
Hide file tree
Showing 6 changed files with 79 additions and 55 deletions.
40 changes: 28 additions & 12 deletions src/core/vdom/helpers/merge-hook.js
Original file line number Diff line number Diff line change
@@ -1,18 +1,34 @@
/* @flow */

export function mergeVNodeHook (def: Object, hookKey: string, hook: Function, key: string) {
key = key + hookKey
const injectedHash: Object = def.__injected || (def.__injected = {})
if (!injectedHash[key]) {
injectedHash[key] = true
const oldHook: ?Function = def[hookKey]
if (oldHook) {
def[hookKey] = function () {
oldHook.apply(this, arguments)
hook.apply(this, arguments)
}
import { remove } from 'shared/util'
import { createFnInvoker } from './update-listeners'

export function mergeVNodeHook (def: Object, hookKey: string, hook: Function) {
let invoker
const oldHook = def[hookKey]

function wrappedHook () {
hook.apply(this, arguments)
// important: remove merged hook to ensure it's called only once
// and prevent memory leak
remove(invoker.fns, wrappedHook)
}

if (!oldHook) {
// no existing hook
invoker = createFnInvoker([wrappedHook])
} else {
/* istanbul ignore if */
if (oldHook.fns && oldHook.merged) {
// already a merged invoker
invoker = oldHook
invoker.fns.push(wrappedHook)
} else {
def[hookKey] = hook
// existing plain hook
invoker = createFnInvoker([oldHook, wrappedHook])
}
}

invoker.merged = true
def[hookKey] = invoker
}
37 changes: 16 additions & 21 deletions src/core/vdom/helpers/update-listeners.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,25 +19,20 @@ const normalizeEvent = cached((name: string): {
}
})

function createEventHandle (fn: Function | Array<Function>): {
fn: Function | Array<Function>;
invoker: Function;
} {
const handle = {
fn,
invoker: function () {
const fn = handle.fn
if (Array.isArray(fn)) {
for (let i = 0; i < fn.length; i++) {
fn[i].apply(null, arguments)
}
} else {
// return handler return value for single handlers
return fn.apply(null, arguments)
export function createFnInvoker (fns: Function | Array<Function>): Function {
function invoker () {
const fns = invoker.fns
if (Array.isArray(fns)) {
for (let i = 0; i < fns.length; i++) {
fns[i].apply(null, arguments)
}
} else {
// return handler return value for single handlers
return fns.apply(null, arguments)
}
}
return handle
invoker.fns = fns
return invoker
}

export function updateListeners (
Expand All @@ -58,19 +53,19 @@ export function updateListeners (
vm
)
} else if (!old) {
if (!cur.invoker) {
cur = on[name] = createEventHandle(cur)
if (!cur.fns) {
cur = on[name] = createFnInvoker(cur)
}
add(event.name, cur.invoker, event.once, event.capture)
add(event.name, cur, event.once, event.capture)
} else if (cur !== old) {
old.fn = cur
old.fns = cur
on[name] = old
}
}
for (name in oldOn) {
if (!on[name]) {
event = normalizeEvent(name)
remove(event.name, oldOn[name].invoker, event.capture)
remove(event.name, oldOn[name], event.capture)
}
}
}
6 changes: 3 additions & 3 deletions src/core/vdom/modules/directives.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
/* @flow */

import { emptyNode } from 'core/vdom/patch'
import { resolveAsset } from 'core/util/options'
import { mergeVNodeHook } from 'core/vdom/helpers/index'
import { emptyNode } from 'core/vdom/patch'

export default {
create: updateDirectives,
Expand Down Expand Up @@ -54,7 +54,7 @@ function _update (oldVnode, vnode) {
}
}
if (isCreate) {
mergeVNodeHook(vnode.data.hook || (vnode.data.hook = {}), 'insert', callInsert, 'dir-insert')
mergeVNodeHook(vnode.data.hook || (vnode.data.hook = {}), 'insert', callInsert)
} else {
callInsert()
}
Expand All @@ -65,7 +65,7 @@ function _update (oldVnode, vnode) {
for (let i = 0; i < dirsWithPostpatch.length; i++) {
callHook(dirsWithPostpatch[i], 'componentUpdated', vnode, oldVnode)
}
}, 'dir-postpatch')
})
}

if (!isCreate) {
Expand Down
15 changes: 7 additions & 8 deletions src/platforms/web/runtime/components/transition.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export function extractTransitionData (comp: Component): Object {
// extract listeners and pass them directly to the transition methods
const listeners: ?Object = options._parentListeners
for (const key in listeners) {
data[camelize(key)] = listeners[key].fn
data[camelize(key)] = listeners[key]
}
return data
}
Expand Down Expand Up @@ -132,11 +132,12 @@ export default {
// component instance. This key will be used to remove pending leaving nodes
// during entering.
const id: string = `__transition-${this._uid}-`
const key: string = child.key = child.key == null
child.key = child.key == null
? id + child.tag
: isPrimitive(child.key)
? (String(child.key).indexOf(id) === 0 ? child.key : id + child.key)
: child.key

const data: Object = (child.data || (child.data = {})).transition = extractTransitionData(this)
const oldRawChild: VNode = this._vnode
const oldChild: VNode = getRealChild(oldRawChild)
Expand All @@ -158,16 +159,14 @@ export default {
mergeVNodeHook(oldData, 'afterLeave', () => {
this._leaving = false
this.$forceUpdate()
}, key)
})
return placeholder(h, rawChild)
} else if (mode === 'in-out') {
let delayedLeave
const performLeave = () => { delayedLeave() }
mergeVNodeHook(data, 'afterEnter', performLeave, key)
mergeVNodeHook(data, 'enterCancelled', performLeave, key)
mergeVNodeHook(oldData, 'delayLeave', leave => {
delayedLeave = leave
}, key)
mergeVNodeHook(data, 'afterEnter', performLeave)
mergeVNodeHook(data, 'enterCancelled', performLeave)
mergeVNodeHook(oldData, 'delayLeave', leave => { delayedLeave = leave })
}
}

Expand Down
1 change: 1 addition & 0 deletions src/platforms/web/runtime/modules/events.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { RANGE_TOKEN, CHECKBOX_RADIO_TOKEN } from 'web/compiler/directives/model
// user-attached handlers.
function normalizeEvents (on) {
let event
/* istanbul ignore if */
if (on[RANGE_TOKEN]) {
// IE input[type=range] only supports `change` event
event = isIE ? 'change' : 'input'
Expand Down
35 changes: 24 additions & 11 deletions src/platforms/web/runtime/modules/transition.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,7 @@ export function enter (vnode: VNodeWithData, toggleDisplay: ?() => void) {
}

const expectsCSS = css !== false && !isIE9
const userWantsControl =
enterHook &&
// enterHook may be a bound method which exposes
// the length of original fn as _length
(enterHook._length || enterHook.length) > 1
const userWantsControl = getHookAgumentsLength(enterHook)

const cb = el._enterCb = once(() => {
if (expectsCSS) {
Expand Down Expand Up @@ -116,7 +112,7 @@ export function enter (vnode: VNodeWithData, toggleDisplay: ?() => void) {
pendingNode.elm._leaveCb()
}
enterHook && enterHook(el, cb)
}, 'transition-insert')
})
}

// start enter transition
Expand Down Expand Up @@ -181,11 +177,7 @@ export function leave (vnode: VNodeWithData, rm: Function) {
} = data

const expectsCSS = css !== false && !isIE9
const userWantsControl =
leave &&
// leave hook may be a bound method which exposes
// the length of original fn as _length
(leave._length || leave.length) > 1
const userWantsControl = getHookAgumentsLength(leave)

const explicitLeaveDuration = isObject(duration) ? duration.leave : duration
if (process.env.NODE_ENV !== 'production' && explicitLeaveDuration != null) {
Expand Down Expand Up @@ -271,6 +263,27 @@ function isValidDuration (val) {
return typeof val === 'number' && !isNaN(val)
}

/**
* Normalize a transition hook's argument length. The hook may be:
* - a merged hook (invoker) with the original in .fns
* - a wrapped component method (check ._length)
* - a plain function (.length)
*/
function getHookAgumentsLength (fn: Function): boolean {
if (!fn) return false
const invokerFns = fn.fns
if (invokerFns) {
// invoker
return getHookAgumentsLength(
Array.isArray(invokerFns)
? invokerFns[0]
: invokerFns
)
} else {
return (fn._length || fn.length) > 1
}
}

function _enter (_: any, vnode: VNodeWithData) {
if (!vnode.data.show) {
enter(vnode)
Expand Down

0 comments on commit 2a5fb41

Please sign in to comment.