From 1579fe84301824cea1f0d2c86971cc0116146dd9 Mon Sep 17 00:00:00 2001 From: Barney Carroll Date: Tue, 29 May 2018 10:53:16 +0100 Subject: [PATCH] Do not normalise component children on ingestion (#2155) * Do not normalise component children on ingestion * Don't normalise vnode children * Component hyperscript tests: children aren't normalised * test, not text * Update change log: #2155 & #2064 --- docs/change-log.md | 1 + render/hyperscript.js | 6 ++-- render/tests/index.html | 1 + render/tests/test-hyperscript.js | 25 +++++++++----- .../tests/test-normalizeComponentChildren.js | 34 +++++++++++++++++++ render/vnode.js | 7 ++-- 6 files changed, 59 insertions(+), 15 deletions(-) create mode 100644 render/tests/test-normalizeComponentChildren.js diff --git a/docs/change-log.md b/docs/change-log.md index dee3406b..0ee241d9 100644 --- a/docs/change-log.md +++ b/docs/change-log.md @@ -19,6 +19,7 @@ #### Breaking changes +- API: Component vnode `children` are not normalized into vnodes on ingestion; normalization only happens if and when they are ingested by the view ([#2155](https://github.com/MithrilJS/mithril.js/pull/2155/) (thanks to [@magikstm](https://github.com/magikstm) for related optimization [#2064](https://github.com/MithrilJS/mithril.js/pull/2064))) - API: `m.redraw()` is always asynchronous ([#1592](https://github.com/MithrilJS/mithril.js/pull/1592)) - API: `m.mount()` will only render its own root when called, it will not trigger a `redraw()` ([#1592](https://github.com/MithrilJS/mithril.js/pull/1592)) - API: Assigning to `vnode.state` (as in `vnode.state = ...`) is no longer supported. Instead, an error is thrown if `vnode.state` changes upon the invocation of a lifecycle hook. diff --git a/render/hyperscript.js b/render/hyperscript.js index 768dad8f..8573559c 100644 --- a/render/hyperscript.js +++ b/render/hyperscript.js @@ -100,12 +100,10 @@ function hyperscript(selector) { while (start < arguments.length) children.push(arguments[start++]) } - var normalized = Vnode.normalizeChildren(children) - if (typeof selector === "string") { - return execSelector(selectorCache[selector] || compileSelector(selector), attrs, normalized) + return execSelector(selectorCache[selector] || compileSelector(selector), attrs, Vnode.normalizeChildren(children)) } else { - return Vnode(selector, attrs.key, attrs, normalized) + return Vnode(selector, attrs.key, attrs, children) } } diff --git a/render/tests/index.html b/render/tests/index.html index eda51921..5c9fc0b7 100644 --- a/render/tests/index.html +++ b/render/tests/index.html @@ -21,6 +21,7 @@ + diff --git a/render/tests/test-hyperscript.js b/render/tests/test-hyperscript.js index f7c5f889..f221bef7 100644 --- a/render/tests/test-hyperscript.js +++ b/render/tests/test-hyperscript.js @@ -551,19 +551,29 @@ o.spec("hyperscript", function() { o.spec("components", function() { o("works with POJOs", function() { var component = { - view: function() { - return m("div") - } + view: function() {} } var vnode = m(component, {id: "a"}, "b") o(vnode.tag).equals(component) o(vnode.attrs.id).equals("a") o(vnode.children.length).equals(1) - o(vnode.children[0].tag).equals("#") - o(vnode.children[0].children).equals("b") + o(vnode.children[0]).equals("b") }) - o("works with functions", function() { + o("works with constructibles", function() { + var component = o.spy() + component.prototype.view = function() {} + + var vnode = m(component, {id: "a"}, "b") + + o(component.callCount).equals(0) + + o(vnode.tag).equals(component) + o(vnode.attrs.id).equals("a") + o(vnode.children.length).equals(1) + o(vnode.children[0]).equals("b") + }) + o("works with closures", function () { var component = o.spy() var vnode = m(component, {id: "a"}, "b") @@ -573,8 +583,7 @@ o.spec("hyperscript", function() { o(vnode.tag).equals(component) o(vnode.attrs.id).equals("a") o(vnode.children.length).equals(1) - o(vnode.children[0].tag).equals("#") - o(vnode.children[0].children).equals("b") + o(vnode.children[0]).equals("b") }) }) }) diff --git a/render/tests/test-normalizeComponentChildren.js b/render/tests/test-normalizeComponentChildren.js new file mode 100644 index 00000000..86f75ffe --- /dev/null +++ b/render/tests/test-normalizeComponentChildren.js @@ -0,0 +1,34 @@ +"use strict" + +var o = require("../../ospec/ospec") +var m = require("../../render/hyperscript") +var domMock = require("../../test-utils/domMock") +var vdom = require("../../render/render") + +o.spec("component children", function () { + var $window = domMock() + var root = $window.document.createElement("div") + var render = vdom($window).render + + o.spec("component children", function () { + var component = { + view: function (vnode) { + return vnode.children + } + } + + var vnode = m(component, "a") + + render(root, vnode) + + o("are not normalized on ingestion", function () { + o(vnode.children[0]).equals("a") + }) + + o("are normalized upon view interpolation", function () { + o(vnode.instance.children.length).equals(1) + o(vnode.instance.children[0].tag).equals("#") + o(vnode.instance.children[0].children).equals("a") + }) + }) +}) diff --git a/render/vnode.js b/render/vnode.js index 13ed393f..5873dca5 100644 --- a/render/vnode.js +++ b/render/vnode.js @@ -8,9 +8,10 @@ Vnode.normalize = function(node) { if (node != null && typeof node !== "object") return Vnode("#", undefined, undefined, node === false ? "" : node, undefined, undefined) return node } -Vnode.normalizeChildren = function normalizeChildren(children) { - for (var i = 0; i < children.length; i++) { - children[i] = Vnode.normalize(children[i]) +Vnode.normalizeChildren = function normalizeChildren(input) { + var children = [] + for (var i = 0; i < input.length; i++) { + children[i] = Vnode.normalize(input[i]) } return children }