Skip to content

Commit 2a5fb41

Browse files
yyx990803yyx990803
committed
re-implement mergeVNodeHook to prevent memory leak (fix vuejs#4990)
1 parent 01b09e6 commit 2a5fb41

File tree

6 files changed

+79
-55
lines changed

6 files changed

+79
-55
lines changed

‎src/core/vdom/helpers/merge-hook.js

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,34 @@
11
/* @flow */
22

3-
export function mergeVNodeHook (def: Object, hookKey: string, hook: Function, key: string) {
4-
key = key + hookKey
5-
const injectedHash: Object = def.__injected || (def.__injected = {})
6-
if (!injectedHash[key]) {
7-
injectedHash[key] = true
8-
const oldHook: ?Function = def[hookKey]
9-
if (oldHook) {
10-
def[hookKey] = function () {
11-
oldHook.apply(this, arguments)
12-
hook.apply(this, arguments)
13-
}
3+
import { remove } from 'shared/util'
4+
import { createFnInvoker } from './update-listeners'
5+
6+
export function mergeVNodeHook (def: Object, hookKey: string, hook: Function) {
7+
let invoker
8+
const oldHook = def[hookKey]
9+
10+
function wrappedHook () {
11+
hook.apply(this, arguments)
12+
// important: remove merged hook to ensure it's called only once
13+
// and prevent memory leak
14+
remove(invoker.fns, wrappedHook)
15+
}
16+
17+
if (!oldHook) {
18+
// no existing hook
19+
invoker = createFnInvoker([wrappedHook])
20+
} else {
21+
/* istanbul ignore if */
22+
if (oldHook.fns && oldHook.merged) {
23+
// already a merged invoker
24+
invoker = oldHook
25+
invoker.fns.push(wrappedHook)
1426
} else {
15-
def[hookKey] = hook
27+
// existing plain hook
28+
invoker = createFnInvoker([oldHook, wrappedHook])
1629
}
1730
}
31+
32+
invoker.merged = true
33+
def[hookKey] = invoker
1834
}

‎src/core/vdom/helpers/update-listeners.js

Lines changed: 16 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -19,25 +19,20 @@ const normalizeEvent = cached((name: string): {
1919
}
2020
})
2121

22-
function createEventHandle (fn: Function | Array<Function>): {
23-
fn: Function | Array<Function>;
24-
invoker: Function;
25-
} {
26-
const handle = {
27-
fn,
28-
invoker: function () {
29-
const fn = handle.fn
30-
if (Array.isArray(fn)) {
31-
for (let i = 0; i < fn.length; i++) {
32-
fn[i].apply(null, arguments)
33-
}
34-
} else {
35-
// return handler return value for single handlers
36-
return fn.apply(null, arguments)
22+
export function createFnInvoker (fns: Function | Array<Function>): Function {
23+
function invoker () {
24+
const fns = invoker.fns
25+
if (Array.isArray(fns)) {
26+
for (let i = 0; i < fns.length; i++) {
27+
fns[i].apply(null, arguments)
3728
}
29+
} else {
30+
// return handler return value for single handlers
31+
return fns.apply(null, arguments)
3832
}
3933
}
40-
return handle
34+
invoker.fns = fns
35+
return invoker
4136
}
4237

4338
export function updateListeners (
@@ -58,19 +53,19 @@ export function updateListeners (
5853
vm
5954
)
6055
} else if (!old) {
61-
if (!cur.invoker) {
62-
cur = on[name] = createEventHandle(cur)
56+
if (!cur.fns) {
57+
cur = on[name] = createFnInvoker(cur)
6358
}
64-
add(event.name, cur.invoker, event.once, event.capture)
59+
add(event.name, cur, event.once, event.capture)
6560
} else if (cur !== old) {
66-
old.fn = cur
61+
old.fns = cur
6762
on[name] = old
6863
}
6964
}
7065
for (name in oldOn) {
7166
if (!on[name]) {
7267
event = normalizeEvent(name)
73-
remove(event.name, oldOn[name].invoker, event.capture)
68+
remove(event.name, oldOn[name], event.capture)
7469
}
7570
}
7671
}

‎src/core/vdom/modules/directives.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
/* @flow */
22

3+
import { emptyNode } from 'core/vdom/patch'
34
import { resolveAsset } from 'core/util/options'
45
import { mergeVNodeHook } from 'core/vdom/helpers/index'
5-
import { emptyNode } from 'core/vdom/patch'
66

77
export default {
88
create: updateDirectives,
@@ -54,7 +54,7 @@ function _update (oldVnode, vnode) {
5454
}
5555
}
5656
if (isCreate) {
57-
mergeVNodeHook(vnode.data.hook || (vnode.data.hook = {}), 'insert', callInsert, 'dir-insert')
57+
mergeVNodeHook(vnode.data.hook || (vnode.data.hook = {}), 'insert', callInsert)
5858
} else {
5959
callInsert()
6060
}
@@ -65,7 +65,7 @@ function _update (oldVnode, vnode) {
6565
for (let i = 0; i < dirsWithPostpatch.length; i++) {
6666
callHook(dirsWithPostpatch[i], 'componentUpdated', vnode, oldVnode)
6767
}
68-
}, 'dir-postpatch')
68+
})
6969
}
7070

7171
if (!isCreate) {

‎src/platforms/web/runtime/components/transition.js

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ export function extractTransitionData (comp: Component): Object {
4747
// extract listeners and pass them directly to the transition methods
4848
const listeners: ?Object = options._parentListeners
4949
for (const key in listeners) {
50-
data[camelize(key)] = listeners[key].fn
50+
data[camelize(key)] = listeners[key]
5151
}
5252
return data
5353
}
@@ -132,11 +132,12 @@ export default {
132132
// component instance. This key will be used to remove pending leaving nodes
133133
// during entering.
134134
const id: string = `__transition-${this._uid}-`
135-
const key: string = child.key = child.key == null
135+
child.key = child.key == null
136136
? id + child.tag
137137
: isPrimitive(child.key)
138138
? (String(child.key).indexOf(id) === 0 ? child.key : id + child.key)
139139
: child.key
140+
140141
const data: Object = (child.data || (child.data = {})).transition = extractTransitionData(this)
141142
const oldRawChild: VNode = this._vnode
142143
const oldChild: VNode = getRealChild(oldRawChild)
@@ -158,16 +159,14 @@ export default {
158159
mergeVNodeHook(oldData, 'afterLeave', () => {
159160
this._leaving = false
160161
this.$forceUpdate()
161-
}, key)
162+
})
162163
return placeholder(h, rawChild)
163164
} else if (mode === 'in-out') {
164165
let delayedLeave
165166
const performLeave = () => { delayedLeave() }
166-
mergeVNodeHook(data, 'afterEnter', performLeave, key)
167-
mergeVNodeHook(data, 'enterCancelled', performLeave, key)
168-
mergeVNodeHook(oldData, 'delayLeave', leave => {
169-
delayedLeave = leave
170-
}, key)
167+
mergeVNodeHook(data, 'afterEnter', performLeave)
168+
mergeVNodeHook(data, 'enterCancelled', performLeave)
169+
mergeVNodeHook(oldData, 'delayLeave', leave => { delayedLeave = leave })
171170
}
172171
}
173172

‎src/platforms/web/runtime/modules/events.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { RANGE_TOKEN, CHECKBOX_RADIO_TOKEN } from 'web/compiler/directives/model
1010
// user-attached handlers.
1111
function normalizeEvents (on) {
1212
let event
13+
/* istanbul ignore if */
1314
if (on[RANGE_TOKEN]) {
1415
// IE input[type=range] only supports `change` event
1516
event = isIE ? 'change' : 'input'

‎src/platforms/web/runtime/modules/transition.js

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -83,11 +83,7 @@ export function enter (vnode: VNodeWithData, toggleDisplay: ?() => void) {
8383
}
8484

8585
const expectsCSS = css !== false && !isIE9
86-
const userWantsControl =
87-
enterHook &&
88-
// enterHook may be a bound method which exposes
89-
// the length of original fn as _length
90-
(enterHook._length || enterHook.length) > 1
86+
const userWantsControl = getHookAgumentsLength(enterHook)
9187

9288
const cb = el._enterCb = once(() => {
9389
if (expectsCSS) {
@@ -116,7 +112,7 @@ export function enter (vnode: VNodeWithData, toggleDisplay: ?() => void) {
116112
pendingNode.elm._leaveCb()
117113
}
118114
enterHook && enterHook(el, cb)
119-
}, 'transition-insert')
115+
})
120116
}
121117

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

183179
const expectsCSS = css !== false && !isIE9
184-
const userWantsControl =
185-
leave &&
186-
// leave hook may be a bound method which exposes
187-
// the length of original fn as _length
188-
(leave._length || leave.length) > 1
180+
const userWantsControl = getHookAgumentsLength(leave)
189181

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

266+
/**
267+
* Normalize a transition hook's argument length. The hook may be:
268+
* - a merged hook (invoker) with the original in .fns
269+
* - a wrapped component method (check ._length)
270+
* - a plain function (.length)
271+
*/
272+
function getHookAgumentsLength (fn: Function): boolean {
273+
if (!fn) return false
274+
const invokerFns = fn.fns
275+
if (invokerFns) {
276+
// invoker
277+
return getHookAgumentsLength(
278+
Array.isArray(invokerFns)
279+
? invokerFns[0]
280+
: invokerFns
281+
)
282+
} else {
283+
return (fn._length || fn.length) > 1
284+
}
285+
}
286+
274287
function _enter (_: any, vnode: VNodeWithData) {
275288
if (!vnode.data.show) {
276289
enter(vnode)

0 commit comments

Comments
 (0)