#510 allow recursive nesting of components

mithril.js

@@ -239,14 +239,22 @@ var m = (function app(window, undefined) {
 			}
 		}
 		else if (data != null && dataType === OBJECT) {
-			if (data.view) {
+			var controllerConstructors = [], controllers = []
-				var module = data, onunload
+			while (data.view) {
-				var controllerConstructor = module.controller.$original || module.controller
+				var controllerConstructor = data.controller.$original || data.controller
-				var controller = controllerConstructor === cached.controllerConstructor ? cached.controller : new (module.controller || function() {})
+				var controllerIndex = cached.controllerConstructors ? cached.controllerConstructors.indexOf(controllerConstructor) : -1
-				data = pendingRequests == 0 ? module.view(controller) : {tag: "placeholder"}
+				var controller = controllerIndex > -1 ? cached.controllers[controllerIndex] : new (data.controller || function() {})
+				var key = data && data.attrs && data.attrs.key
+				data = pendingRequests == 0 ? data.view(controller) : {tag: "placeholder"}
+				if (key) {
+					if (!data.attrs) data.attrs = {}
+					data.attrs.key = key
+				}
 				if (controller.onunload) unloaders.push({controller: controller, handler: controller.onunload})
-				if (!data.tag) throw new Error("Component template must return a virtual element, not an array, string, etc.")
+				controllerConstructors.push(controllerConstructor)
+				controllers.push(controller)
 			}
+			if (!data.tag && controllers.length) throw new Error("Component template must return a virtual element, not an array, string, etc.")
 			if (!data.attrs) data.attrs = {};
 			if (!cached.attrs) cached.attrs = {};
 
@@ -256,7 +264,11 @@ var m = (function app(window, undefined) {
 			if (data.tag != cached.tag || dataAttrKeys.join() != Object.keys(cached.attrs).join() || data.attrs.id != cached.attrs.id || data.attrs.key != cached.attrs.key || (m.redraw.strategy() == "all" && cached.configContext && cached.configContext.retain !== true) || (m.redraw.strategy() == "diff" && cached.configContext && cached.configContext.retain === false)) {
 				if (cached.nodes.length) clear(cached.nodes);
 				if (cached.configContext && typeof cached.configContext.onunload === FUNCTION) cached.configContext.onunload()
-				if (cached.controller && typeof cached.controller.onunload === FUNCTION) cached.controller.onunload({preventDefault: function() {}})
+				if (cached.controllers) {
+					for (var i = 0, controller; controller = cached.controllers[i]; i++) {
+						if (typeof controller.onunload === FUNCTION) controller.onunload({preventDefault: function() {}})
+					}
+				}
 			}
 			if (type.call(data.tag) != STRING) return;
 
@@ -277,14 +289,16 @@ var m = (function app(window, undefined) {
 						data.children,
 					nodes: [node]
 				};
-				if (controller) {
+				if (controllers.length) {
-					cached.controllerConstructor = controllerConstructor
+					cached.controllerConstructors = controllerConstructors
-					cached.controller = controller
+					cached.controllers = controllers
-					if (controller.onunload && controller.onunload.$old) controller.onunload = controller.onunload.$old
+					for (var i = 0, controller; controller = controllers[i]; i++) {
-					if (pendingRequests && controller.onunload) {
+						if (controller.onunload && controller.onunload.$old) controller.onunload = controller.onunload.$old
-						var onunload = controller.onunload
+						if (pendingRequests && controller.onunload) {
-						controller.onunload = function() {}
+							var onunload = controller.onunload
-						controller.onunload.$old = onunload
+							controller.onunload = function() {}
+							controller.onunload.$old = onunload
+						}
 					}
 				}
 				
@@ -298,9 +312,9 @@ var m = (function app(window, undefined) {
 				if (hasKeys) setAttributes(node, data.tag, data.attrs, cached.attrs, namespace);
 				cached.children = build(node, data.tag, undefined, undefined, data.children, cached.children, false, 0, data.attrs.contenteditable ? node : editable, namespace, configs);
 				cached.nodes.intact = true;
-				if (controller) {
+				if (controllers.length) {
-					cached.controllerConstructor = controllerConstructor
+					cached.controllerConstructors = controllerConstructors
-					cached.controller = controller
+					cached.controllers = controllers
 				}
 				if (shouldReattach === true && node != null) parentElement.insertBefore(node, parentElement.childNodes[index] || null)
 			}
@@ -427,7 +441,11 @@ var m = (function app(window, undefined) {
 			cached.configContext.onunload();
 			cached.configContext.onunload = null
 		}
-		if (cached.controller && typeof cached.controller.onunload === FUNCTION) cached.controller.onunload({preventDefault: function() {}});
+		if (cached.controllers) {
+			for (var i = 0, controller; controller = cached.controllers[i]; i++) {
+				if (typeof controller.onunload === FUNCTION) controller.onunload({preventDefault: function() {}});
+			}
+		}
 		if (cached.children) {
 			if (type.call(cached.children) === ARRAY) {
 				for (var i = 0, child; child = cached.children[i]; i++) unload(child)
@@ -536,9 +554,7 @@ var m = (function app(window, undefined) {
 		}
 		var view = function(ctrl) {
 			if (arguments.length > 1) args = args.concat([].slice.call(arguments, 1))
-			var template = module.view.apply(module, args ? [ctrl].concat(args) : [ctrl])
+			return module.view.apply(module, args ? [ctrl].concat(args) : [ctrl])
-			if (args[0] && args[0].key != null) template.attrs.key = args[0].key
-			return template
 		}
 		controller.$original = module.controller
 		var output = {controller: controller, view: view}

tests/mithril-tests.js

@@ -195,6 +195,45 @@ function testMithril(mock) {
 		
 		return count1 == 1 && count2 == 2
 	})
+	test(function() {
+		//sub component controller should only run once
+		mock.requestAnimationFrame.$resolve()
+
+		var root = mock.document.createElement("div")
+		var count1 = 0, count2 = 0, count3 = 0, count4 = 0
+		var module = {
+			view: function(ctrl) {
+				return m.module(sub)
+			}
+		}
+		var sub = {
+			controller: function() {
+				count1++
+			},
+			view: function(ctrl) {
+				count2++
+				return subsub
+			}
+		}
+		var subsub = {
+			controller: function() {
+				count3++
+			},
+			view: function(ctrl) {
+				count4++
+				return m("div", "test")
+			}
+		}
+		m.module(root, module)
+
+		mock.requestAnimationFrame.$resolve()
+		
+		m.redraw(true)
+
+		mock.requestAnimationFrame.$resolve()
+		
+		return count1 == 1 && count2 == 2 && count3 == 1 && count4 == 2
+	})
 	test(function() {
 		//keys in components should work
 		mock.requestAnimationFrame.$resolve()
@@ -230,6 +269,46 @@ function testMithril(mock) {
 		
 		return firstBefore === firstAfter
 	})
+	test(function() {
+		//keys in subcomponents should work
+		mock.requestAnimationFrame.$resolve()
+
+		var root = mock.document.createElement("div")
+		var list = [1, 2, 3]
+		var module = {
+			controller: function() {},
+			view: function(ctrl) {
+				return list.map(function(i) {
+					return m.module(sub, {key: i})
+				})
+			}
+		}
+		var sub = {
+			view: function() {
+				return m.module(subsub)
+			}
+		}
+		var subsub = {
+			controller: function() {},
+			view: function() {
+				return m("div")
+			}
+		}
+		m.module(root, module)
+		
+		var firstBefore = root.childNodes[0]
+		
+		mock.requestAnimationFrame.$resolve()
+		
+		list.reverse()
+		m.redraw(true)
+
+		mock.requestAnimationFrame.$resolve()
+		
+		var firstAfter = root.childNodes[2]
+		
+		return firstBefore === firstAfter
+	})
 	test(function() {
 		//keys in components should work even if component internally messes them up
 		mock.requestAnimationFrame.$resolve()
@@ -265,6 +344,47 @@ function testMithril(mock) {
 		
 		return firstBefore === firstAfter
 	})
+	test(function() {
+		//keys in subcomponents should work even if component internally messes them up
+		mock.requestAnimationFrame.$resolve()
+
+		var root = mock.document.createElement("div")
+		var list = [1, 2, 3]
+		var module = {
+			controller: function() {},
+			view: function(ctrl) {
+				return list.map(function(i) {
+					return m.module(sub, {key: i})
+				})
+			}
+		}
+		var sub = {
+			controller: function() {},
+			view: function() {
+				return subsub
+			}
+		}
+		var subsub = {
+			controller: function() {},
+			view: function() {
+				return m("div", {key: 1})
+			}
+		}
+		m.module(root, module)
+		
+		var firstBefore = root.childNodes[0]
+		
+		mock.requestAnimationFrame.$resolve()
+		
+		list.reverse()
+		m.redraw(true)
+
+		mock.requestAnimationFrame.$resolve()
+		
+		var firstAfter = root.childNodes[2]
+		
+		return firstBefore === firstAfter
+	})
 	test(function() {
 		//component identity should stay intact if components are descendants of keyed elements
 		mock.requestAnimationFrame.$resolve()
@@ -300,6 +420,47 @@ function testMithril(mock) {
 		
 		return firstBefore === firstAfter
 	})
+	test(function() {
+		//subcomponent identity should stay intact if components are descendants of keyed elements
+		mock.requestAnimationFrame.$resolve()
+
+		var root = mock.document.createElement("div")
+		var list = [1, 2, 3]
+		var module = {
+			controller: function() {},
+			view: function(ctrl) {
+				return list.map(function(i) {
+					return m("div", {key: i}, m.module(sub))
+				})
+			}
+		}
+		var sub = {
+			controller: function() {},
+			view: function() {
+				return subsub
+			}
+		}
+		var subsub = {
+			controller: function() {},
+			view: function() {
+				return m("div")
+			}
+		}
+		m.module(root, module)
+		
+		var firstBefore = root.childNodes[0].childNodes[0]
+		
+		mock.requestAnimationFrame.$resolve()
+		
+		list.reverse()
+		m.redraw(true)
+
+		mock.requestAnimationFrame.$resolve()
+		
+		var firstAfter = root.childNodes[2].childNodes[0]
+		
+		return firstBefore === firstAfter
+	})
 	test(function() {
 		//component should call onunload when removed from template
 		mock.requestAnimationFrame.$resolve()
@@ -338,6 +499,54 @@ function testMithril(mock) {
 		
 		return unloaded === 3
 	})
+	test(function() {
+		//subcomponent should call onunload when removed from template
+		mock.requestAnimationFrame.$resolve()
+
+		var root = mock.document.createElement("div")
+		var list = [1, 2, 3]
+		var unloaded1, unloaded2
+		var module = {
+			controller: function() {},
+			view: function(ctrl) {
+				return list.map(function(i) {
+					return m.module(sub, {key: i})
+				})
+			}
+		}
+		var sub = {
+			controller: function(opts) {
+				this.onunload = function() {
+					unloaded1 = opts.key
+				}
+			},
+			view: function(ctrl, opts) {
+				return m.module(subsub, {key: opts.key})
+			}
+		}
+		var subsub = {
+			controller: function(opts) {
+				this.onunload = function() {
+					unloaded2 = opts.key
+				}
+			},
+			view: function() {
+				return m("div")
+			}
+		}
+		m.module(root, module)
+		
+		var firstBefore = root.childNodes[0]
+		
+		mock.requestAnimationFrame.$resolve()
+		
+		list.pop()
+		m.redraw(true)
+
+		mock.requestAnimationFrame.$resolve()
+		
+		return unloaded1 === 3 && unloaded2 === 3
+	})
 	test(function() {
 		//calling m.redraw synchronously from controller constructor should not trigger extra redraws
 		mock.requestAnimationFrame.$resolve()
@@ -365,6 +574,39 @@ function testMithril(mock) {
 		
 		return count === 1
 	})
+	test(function() {
+		//calling m.redraw synchronously from controller constructor should not trigger extra redraws
+		mock.requestAnimationFrame.$resolve()
+
+		var root = mock.document.createElement("div")
+		var count = 0
+		var module = {
+			controller: function() {},
+			view: function(ctrl) {
+				return m.module(sub)
+			}
+		}
+		var sub = {
+			controller: function(opts) {},
+			view: function() {
+				return subsub
+			}
+		}
+		var subsub = {
+			controller: function(opts) {
+				m.redraw()
+			},
+			view: function() {
+				count++
+				return m("div")
+			}
+		}
+		m.module(root, module)
+		
+		mock.requestAnimationFrame.$resolve()
+		
+		return count === 1
+	})
 	test(function() {
 		//calling preventDefault from component's onunload should prevent route change
 		mock.requestAnimationFrame.$resolve()
@@ -402,6 +644,50 @@ function testMithril(mock) {
 		
 		return loaded === false
 	})
+	test(function() {
+		//calling preventDefault from subcomponent's onunload should prevent route change
+		mock.requestAnimationFrame.$resolve()
+		mock.location.search = "?"
+
+		var root = mock.document.createElement("div")
+		var loaded = false
+		var testEnabled = true
+		var module = {
+			controller: function() {},
+			view: function() {
+				return m.module(sub)
+			}
+		}
+		var sub = {
+			controller: function(opts) {
+			},
+			view: function() {
+				return m.module(subsub)
+			}
+		}
+		var subsub = {
+			controller: function(opts) {
+				controller = this
+				this.onunload = function(e) {if (testEnabled) e.preventDefault()}
+			},
+			view: function() {
+				return m("div")
+			}
+		}
+		m.route(root, "/a", {
+			"/a": module,
+			"/b": {controller: function() {loaded = true}, view: function() {}}
+		})
+		
+		mock.requestAnimationFrame.$resolve()
+		
+		m.route("/b")
+		
+		mock.requestAnimationFrame.$resolve()
+		testEnabled = false
+		
+		return loaded === false
+	})
 	test(function() {
 		//calling preventDefault from non-curried component's onunload should prevent route change
 		mock.requestAnimationFrame.$resolve()
@@ -439,6 +725,49 @@ function testMithril(mock) {
 		
 		return loaded === false
 	})
+	test(function() {
+		//calling preventDefault from non-curried subcomponent's onunload should prevent route change
+		mock.requestAnimationFrame.$resolve()
+		mock.location.search = "?"
+
+		var root = mock.document.createElement("div")
+		var loaded = false
+		var testEnabled = true
+		var module = {
+			controller: function() {},
+			view: function() {
+				return sub
+			}
+		}
+		var sub = {
+			controller: function(opts) {},
+			view: function() {
+				return subsub
+			}
+		}
+		var subsub = {
+			controller: function(opts) {
+				controller = this
+				this.onunload = function(e) {if (testEnabled) e.preventDefault()}
+			},
+			view: function() {
+				return m("div")
+			}
+		}
+		m.route(root, "/a", {
+			"/a": module,
+			"/b": {controller: function() {loaded = true}, view: function() {}}
+		})
+		
+		mock.requestAnimationFrame.$resolve()
+		
+		m.route("/b")
+		
+		mock.requestAnimationFrame.$resolve()
+		testEnabled = false
+		
+		return loaded === false
+	})
 	test(function() {
 		// nested modules under keyed components should render
 		mock.requestAnimationFrame.$resolve()