Address #2021 review comments

This commit is contained in:
Pierre-Yves Gérardy 2017-12-04 14:33:21 +01:00 committed by Pierre-Yves Gérardy
parent 3f37d3d7c0
commit 9f09ac069c
2 changed files with 13 additions and 16 deletions

View file

@ -189,8 +189,9 @@ module.exports = function($window) {
// The updateNodes() function: // The updateNodes() function:
// - deals with trivial cases // - deals with trivial cases
// - determines whether the lists are keyed or unkeyed // - determines whether the lists are keyed or unkeyed
// (we take advantage of the fact that mixed diff is not supported and settle on the // (Currently we look for the first pair of non-null nodes and deem the lists unkeyed
// keyedness of the first vnode we find) // if both nodes are unkeyed. TODO (v2) We may later take advantage of the fact that
// mixed diff is not supported and settle on the keyedness of the first vnode we find)
// - diffs them and patches the DOM if needed (that's the brunt of the code) // - diffs them and patches the DOM if needed (that's the brunt of the code)
// - manages the leftovers: after diffing, are there: // - manages the leftovers: after diffing, are there:
// - old nodes left to remove? // - old nodes left to remove?
@ -258,7 +259,7 @@ module.exports = function($window) {
// in three circumstances: // in three circumstances:
// - If the old and the new vnodes are the same object (`===`), diff is not performed unless // - If the old and the new vnodes are the same object (`===`), diff is not performed unless
// the old node comes from the pool (since it must be recycled/re-created). // the old node comes from the pool (since it must be recycled/re-created).
// - The value of `oFromPool` is passed as the `recycling` parameter of `upateNode()` (whether // - The value of `oFromPool` is passed as the `recycling` parameter of `updateNode()` (whether
// the parent is being recycled is also factred in here) // the parent is being recycled is also factred in here)
// - It is used in the DOM node insertion logic (see below) // - It is used in the DOM node insertion logic (see below)
// //
@ -284,19 +285,15 @@ module.exports = function($window) {
else if (old == null) createNodes(parent, vnodes, 0, vnodes.length, hooks, nextSibling, ns) else if (old == null) createNodes(parent, vnodes, 0, vnodes.length, hooks, nextSibling, ns)
else if (vnodes == null) removeNodes(old, 0, old.length, vnodes, recyclingParent) else if (vnodes == null) removeNodes(old, 0, old.length, vnodes, recyclingParent)
else { else {
var start = 0, commonLength = Math.min(old.length, vnodes.length), originalOldLength = old.length, hasPool = false, isUnkeyed var start = 0, commonLength = Math.min(old.length, vnodes.length), originalOldLength = old.length, hasPool = false, isUnkeyed = false
for(; start < commonLength; start++) { for(; start < commonLength; start++) {
if (old[start] != null) { if (old[start] != null && vnodes[start] != null) {
isUnkeyed = old[start].key == null if (old[start].key == null && vnodes[start].key == null) isUnkeyed = true
break
}
if (vnodes[start] != null) {
isUnkeyed = vnodes[start].key == null
break break
} }
} }
if (isUnkeyed && originalOldLength === vnodes.length) { if (isUnkeyed && originalOldLength === vnodes.length) {
for (; start < originalOldLength; start++) { for (start = 0; start < originalOldLength; start++) {
if (old[start] === vnodes[start] && !recyclingParent || old[start] == null && vnodes[start] == null) continue if (old[start] === vnodes[start] && !recyclingParent || old[start] == null && vnodes[start] == null) continue
else if (old[start] == null) createNode(parent, vnodes[start], hooks, ns, getNextSibling(old, start + 1, originalOldLength, nextSibling)) else if (old[start] == null) createNode(parent, vnodes[start], hooks, ns, getNextSibling(old, start + 1, originalOldLength, nextSibling))
else if (vnodes[start] == null) removeNodes(old, start, start + 1, vnodes, recyclingParent) else if (vnodes[start] == null) removeNodes(old, start, start + 1, vnodes, recyclingParent)
@ -310,7 +307,7 @@ module.exports = function($window) {
old = old.concat(old.pool) old = old.concat(old.pool)
} }
var oldStart = start, oldEnd = old.length - 1, end = vnodes.length - 1, map, o, v, oFromPool var oldStart = start = 0, oldEnd = old.length - 1, end = vnodes.length - 1, map, o, v, oFromPool
while (oldEnd >= oldStart && end >= start) { while (oldEnd >= oldStart && end >= start) {
o = old[oldStart] o = old[oldStart]
@ -318,12 +315,12 @@ module.exports = function($window) {
oFromPool = hasPool && oldStart >= originalOldLength oFromPool = hasPool && oldStart >= originalOldLength
if (o === v && !oFromPool && !recyclingParent || o == null && v == null) oldStart++, start++ if (o === v && !oFromPool && !recyclingParent || o == null && v == null) oldStart++, start++
else if (o == null) { else if (o == null) {
if (isUnkeyed) { if (isUnkeyed || v.key == null) {
createNode(parent, vnodes[start], hooks, ns, getNextSibling(old, ++start, originalOldLength, nextSibling)) createNode(parent, vnodes[start], hooks, ns, getNextSibling(old, ++start, originalOldLength, nextSibling))
} }
oldStart++ oldStart++
} else if (v == null) { } else if (v == null) {
if (isUnkeyed){ if (isUnkeyed || o.key == null) {
removeNodes(old, start, start + 1, vnodes, recyclingParent) removeNodes(old, start, start + 1, vnodes, recyclingParent)
oldStart++ oldStart++
} }
@ -568,8 +565,8 @@ module.exports = function($window) {
// not fire. // not fire.
function removeNode(vnode, context, recycling) { function removeNode(vnode, context, recycling) {
var expected = 1, called = 0 var expected = 1, called = 0
var original = vnode.state
if (!recycling) { if (!recycling) {
var original = vnode.state
if (vnode.attrs && typeof vnode.attrs.onbeforeremove === "function") { if (vnode.attrs && typeof vnode.attrs.onbeforeremove === "function") {
var result = callHook.call(vnode.attrs.onbeforeremove, vnode) var result = callHook.call(vnode.attrs.onbeforeremove, vnode)
if (result != null && typeof result.then === "function") { if (result != null && typeof result.then === "function") {

View file

@ -195,7 +195,7 @@ o.spec("onremove", function() {
o("onremove doesn't fire on nodes that go from pool to pool (#1990)", function() { o("onremove doesn't fire on nodes that go from pool to pool (#1990)", function() {
var onremove = o.spy(); var onremove = o.spy();
render(root, [m("div", m("div")), m("div", m("div", {onremove: onremove}))]); render(root, [m("div", m("div")), m("div", m("div", {onremove: onremove}))]);
render(root, [m("div", m("div"))]); render(root, [m("div", m("div"))]);
render(root, []); render(root, []);