From 9f110774aaeb7c692e7a387dcdee0f59237cb3b7 Mon Sep 17 00:00:00 2001 From: Leo Horie Date: Tue, 24 Mar 2015 22:18:06 -0400 Subject: [PATCH] #510 allow recursive nesting of components --- mithril.js | 60 +++++--- tests/mithril-tests.js | 329 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 367 insertions(+), 22 deletions(-) diff --git a/mithril.js b/mithril.js index 8d93e603..06e155e0 100644 --- a/mithril.js +++ b/mithril.js @@ -239,14 +239,22 @@ var m = (function app(window, undefined) { } } else if (data != null && dataType === OBJECT) { - if (data.view) { - var module = data, onunload - var controllerConstructor = module.controller.$original || module.controller - var controller = controllerConstructor === cached.controllerConstructor ? cached.controller : new (module.controller || function() {}) - data = pendingRequests == 0 ? module.view(controller) : {tag: "placeholder"} + var controllerConstructors = [], controllers = [] + while (data.view) { + var controllerConstructor = data.controller.$original || data.controller + var controllerIndex = cached.controllerConstructors ? cached.controllerConstructors.indexOf(controllerConstructor) : -1 + 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) { - cached.controllerConstructor = controllerConstructor - cached.controller = controller - if (controller.onunload && controller.onunload.$old) controller.onunload = controller.onunload.$old - if (pendingRequests && controller.onunload) { - var onunload = controller.onunload - controller.onunload = function() {} - controller.onunload.$old = onunload + if (controllers.length) { + cached.controllerConstructors = controllerConstructors + cached.controllers = controllers + for (var i = 0, controller; controller = controllers[i]; i++) { + if (controller.onunload && controller.onunload.$old) controller.onunload = controller.onunload.$old + if (pendingRequests && controller.onunload) { + var onunload = controller.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) { - cached.controllerConstructor = controllerConstructor - cached.controller = controller + if (controllers.length) { + cached.controllerConstructors = controllerConstructors + 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]) - if (args[0] && args[0].key != null) template.attrs.key = args[0].key - return template + return module.view.apply(module, args ? [ctrl].concat(args) : [ctrl]) } controller.$original = module.controller var output = {controller: controller, view: view} diff --git a/tests/mithril-tests.js b/tests/mithril-tests.js index 3c24e130..b6926e79 100644 --- a/tests/mithril-tests.js +++ b/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()