render: extract pool addtition logic, don't run onremove on nodes that move from pool to pool (fix #1990)

render/render.js

@@ -185,7 +185,7 @@ module.exports = function($window) {
 				}
 			}
 			recycling = recycling || isRecyclable(old, vnodes)
-			if (recycling) {
+			if (recycling && old.pool != null) {
 				var pool = old.pool
 				old = old.concat(old.pool)
 			}
@@ -250,7 +250,14 @@ module.exports = function($window) {
 				if (end < start) break
 			}
 			createNodes(parent, vnodes, start, end + 1, hooks, nextSibling, ns)
-			removeNodes(old, oldStart, oldEnd + 1, vnodes)
+			removeNodes(old, oldStart, Math.min(oldEnd + 1, pool == null ? old.length : old.length - pool.length), vnodes)
+			if (pool != null) {
+				var limit = Math.max(oldStart, old.length - pool.length)
+				for (; oldEnd >= limit; oldEnd--) {
+					if (old[oldEnd].skip) old[oldEnd].skip = false
+					else addToPool(old[oldEnd], vnodes)
+				}
+			}
 		}
 	}
 	function updateNode(parent, old, vnode, hooks, nextSibling, recycling, ns) {
@@ -455,10 +462,7 @@ module.exports = function($window) {
 						}
 					}
 					removeNodeFromDOM(vnode.dom)
-					if (context != null && vnode.domSize == null && !hasIntegrationMethods(vnode.attrs) && typeof vnode.tag === "string") { //TODO test custom elements
+					addToPool(vnode, context)
-						if (!context.pool) context.pool = [vnode]
-						else context.pool.push(vnode)
-					}
 				}
 			}
 		}
@@ -467,6 +471,12 @@ module.exports = function($window) {
 		var parent = node.parentNode
 		if (parent != null) parent.removeChild(node)
 	}
+	function addToPool(vnode, context) {
+		if (context != null && vnode.domSize == null && !hasIntegrationMethods(vnode.attrs) && typeof vnode.tag === "string") { //TODO test custom elements
+			if (!context.pool) context.pool = [vnode]
+			else context.pool.push(vnode)
+		}
+	}
 	function onremove(vnode) {
 		if (vnode.attrs && typeof vnode.attrs.onremove === "function") callHook.call(vnode.attrs.onremove, vnode)
 		if (typeof vnode.tag !== "string") {

render/tests/test-updateNodes.js

@@ -839,6 +839,19 @@ o.spec("updateNodes", function() {
 		o(root.childNodes[0].nodeName).equals("A")
 		o(root.childNodes[1].nodeName).equals("B")
 	})
+	o("onremove doesn't fire from nodes in the pool (#1990)", function () {
+		var onremove = o.spy()
+		render(root, [
+			{tag: "div", children: [{tag: "div", attrs: {onremove: onremove}}]},
+			{tag: "div", children: [{tag: "div", attrs: {onremove: onremove}}]}
+		])
+		render(root, [
+			{tag: "div", children: [{tag: "div", attrs: {onremove: onremove}}]}
+		])
+		render(root,[])
+
+		o(onremove.callCount).equals(2)
+	})
 	o("cached, non-keyed nodes skip diff", function () {
 		var onupdate = o.spy();
 		var cached = {tag:"a", attrs:{onupdate: onupdate}}
@@ -857,6 +870,72 @@ o.spec("updateNodes", function() {
 
 		o(onupdate.callCount).equals(0)
 	})
+	o("keyed cached elements are re-initialized when brought back from the pool", function () {
+		var onupdate = o.spy()
+		var oncreate = o.spy()
+		var cached = {
+			tag: "B", key: 1, children: [
+				{tag: "A", attrs: {oncreate: oncreate, onupdate: onupdate}, text: "A"}
+			]
+		}
+		render(root, [{tag: "div", children: [cached]}])
+		render(root, [])
+		render(root, [{tag: "div", children: [cached]}])
+
+		o(oncreate.callCount).equals(2)
+		o(onupdate.callCount).equals(0)
+	})
+
+	o("uneyed cached elements are re-initialized when brought back from the pool", function () {
+		var onupdate = o.spy()
+		var oncreate = o.spy()
+		var cached = {
+			tag: "B", children: [
+				{tag: "A", attrs: {oncreate: oncreate, onupdate: onupdate}, text: "A"}
+			]
+		}
+		render(root, [{tag: "div", children: [cached]}])
+		render(root, [])
+		render(root, [{tag: "div", children: [cached]}])
+
+		o(oncreate.callCount).equals(2)
+		o(onupdate.callCount).equals(0)
+	})
+
+	o("keyed cached elements are re-initialized when brought back from nested pools", function () {
+		var onupdate = o.spy()
+		var oncreate = o.spy()
+		var cached = {
+			tag: "B", key: 1, children: [
+				{tag: "A", attrs: {oncreate: oncreate, onupdate: onupdate}, text: "A"}
+			]
+		}
+		render(root, [{tag: "div", children: [cached]}])
+		render(root, [{tag: "div", children: []}])
+		render(root, [])
+		render(root, [{tag: "div", children: [cached]}])
+
+		o(oncreate.callCount).equals(2)
+		o(onupdate.callCount).equals(0)
+	})
+
+	o("unkeyed cached elements are re-initialized when brought back from nested pools", function () {
+		var onupdate = o.spy()
+		var oncreate = o.spy()
+		var cached = {
+			tag: "B", children: [
+				{tag: "A", attrs: {oncreate: oncreate, onupdate: onupdate}, text: "A"}
+			]
+		}
+		render(root, [{tag: "div", children: [cached]}])
+		render(root, [{tag: "div", children: []}])
+		render(root, [])
+		render(root, [{tag: "div", children: [cached]}])
+
+		o(oncreate.callCount).equals(2)
+		o(onupdate.callCount).equals(0)
+	})
+
 	o("null stays in place", function() {
 		var create = o.spy()
 		var update = o.spy()