From 6c562d2b9bee65301969a3e4a36458968b81e0fb Mon Sep 17 00:00:00 2001 From: Isiah Meadows Date: Wed, 3 Jul 2019 17:05:44 -0400 Subject: [PATCH] Fix keys, normalize holes (#2452) * Fix #2434 * Treat holes as unkeyed, normalize boolean/null/undefined This brings a lot better consistency with that API, even though it's slightly breaking. (I had to update a bunch of tests to correspond with it.) * Fill in PR number [skip ci] --- docs/change-log.md | 10 ++ docs/hyperscript.md | 2 +- docs/keys.md | 25 ++++ render/hyperscript.js | 2 +- render/render.js | 161 +++++++++++-------------- render/tests/test-component.js | 15 +-- render/tests/test-fragment.js | 52 +++----- render/tests/test-hyperscript.js | 36 +++--- render/tests/test-normalize.js | 14 +-- render/tests/test-normalizeChildren.js | 5 +- render/tests/test-updateNodes.js | 55 ++++++--- render/vnode.js | 5 +- 12 files changed, 200 insertions(+), 182 deletions(-) diff --git a/docs/change-log.md b/docs/change-log.md index 29f3a8c5..f4510f85 100644 --- a/docs/change-log.md +++ b/docs/change-log.md @@ -45,6 +45,15 @@ - Previously, it was only set for all non-`GET` methods and only when `useBody: true` was passed (the default), and it was always set for them. Now it's automatically omitted when no body is present, so the hole is slightly broadened. - route: query parameters in hash strings are no longer supported ([#2448](https://github.com/MithrilJS/mithril.js/pull/2448) [@isiahmeadows](https://github.com/isiahmeadows)) - It's technically invalid in hashes, so I'd rather push people to keep in line with spec. +- render: validate all elements are either keyed or unkeyed, and treat `null`/`undefined`/booleans as strictly unkeyed ([#2452](https://github.com/MithrilJS/mithril.js/pull/2452) [@isiahmeadows](https://github.com/isiahmeadows)) + - Gives a nice little perf boost with keyed fragments. + - Minor, but imperceptible impact (within the margin of error) with unkeyed fragments. + - Also makes the model a lot more consistent - all values are either keyed or unkeyed. +- vnodes: normalize boolean children to `null`/`undefined` at the vnode level, always stringify non-object children that aren't holes ([#2452](https://github.com/MithrilJS/mithril.js/pull/2452) [@isiahmeadows](https://github.com/isiahmeadows)) + - Previously, `true` was equivalent to `"true"` and `false` was equivalent to `""`. + - Previously, numeric children weren't coerced. Now, they are. + - Unlikely to break most components, but it *could* break some users. + - This increases consistency with how booleans are handled with children, so it should be more intuitive. #### News @@ -101,6 +110,7 @@ - route: arbitrary prefixes are properly supported now, including odd prefixes like `?#` and invalid prefixes like `#foo#bar` ([#2448](https://github.com/MithrilJS/mithril.js/pull/2448) [@isiahmeadows](https://github.com/isiahmeadows)) - request: correct IE workaround for response type non-support ([#2449](https://github.com/MithrilJS/mithril.js/pull/2449) [@isiahmeadows](https://github.com/isiahmeadows)) - render: correct `contenteditable` check to also check for `contentEditable` property name ([#2450](https://github.com/MithrilJS/mithril.js/pull/2450) [@isiahmeadows](https://github.com/isiahmeadows)) +- docs: clarify valid key usage ([#2452](https://github.com/MithrilJS/mithril.js/pull/2452) [@isiahmeadows](https://github.com/isiahmeadows)) --- diff --git a/docs/hyperscript.md b/docs/hyperscript.md index bd718b1c..19a70b27 100644 --- a/docs/hyperscript.md +++ b/docs/hyperscript.md @@ -159,7 +159,7 @@ m("button", { If the value of such an attribute is `null` or `undefined`, it is treated as if the attribute was absent. -If there are class names in both first and second arguments of `m()`, they are merged together as you would expect. If the value of the class in the second argument is `null`or `undefined`, it is ignored. +If there are class names in both first and second arguments of `m()`, they are merged together as you would expect. If the value of the class in the second argument is `null` or `undefined`, it is ignored. If another attribute is present in both the first and the second argument, the second one takes precedence even if it is is `null` or `undefined`. diff --git a/docs/keys.md b/docs/keys.md index 377e65ba..8319350e 100644 --- a/docs/keys.md +++ b/docs/keys.md @@ -75,6 +75,27 @@ function correctUserList(users) { } ``` +Also, you might want to reinitialize a component. You can use the common pattern of a single-item keyed fragment where you change the key to destroy and reinitialize the element. + +```javascript +function ResettableToggle() { + var toggleKey = false + + function reset() { + toggleKey = !toggleKey + } + + return { + view: function() { + return [ + m("button", {onclick: reset}, "Reset toggle"), + [m(Toggle, {key: toggleKey})] + ] + } + } +} +``` + --- ### Debugging key related issues @@ -178,6 +199,10 @@ m("div", [ ]) ``` +In fact, this will cause an error to be thrown, to remind you to not do it. So don't do it! + +Note that `null`s, `undefined`s, and booleans are considered unkeyed nodes. If you want the keyed equivalent, use `m.fragment({key: ...}, [])`, a keyed empty fragment. + #### Avoid passing model data directly to components if the model uses `key` as a data property The `key` property may appear in your data model in a way that conflicts with Mithril's key logic. For example, a component may represent an entity whose `key` property is expected to change over time. This can lead to components receiving the wrong data, re-initialize, or change positions unexpectedly. If your data model uses the `key` property, make sure to wrap the data such that Mithril doesn't misinterpret it as a rendering instruction: diff --git a/render/hyperscript.js b/render/hyperscript.js index ff793e43..b4346581 100644 --- a/render/hyperscript.js +++ b/render/hyperscript.js @@ -93,7 +93,7 @@ function hyperscript(selector) { vnode.children = Vnode.normalizeChildren(vnode.children) if (selector !== "[") return execSelector(selectorCache[selector] || compileSelector(selector), vnode) } - + vnode.tag = selector return vnode } diff --git a/render/render.js b/render/render.js index c818193a..c54ba654 100644 --- a/render/render.js +++ b/render/render.js @@ -44,8 +44,24 @@ module.exports = function($window) { return null } } + function validateKeys(vnodes, isKeyed) { + // Note: this is a *very* perf-sensitive check. + // Fun fact: merging the loop like this is somehow faster than splitting + // it, noticeably so. + for (var i = 1; i < vnodes.length; i++) { + if ((vnodes[i] != null && vnodes[i].key != null) !== isKeyed) { + throw new TypeError("Vnodes must either always have keys or never have keys!") + } + } + } //create - function createNodes(parent, vnodes, start, end, hooks, nextSibling, ns) { + function createNodesChecked(parent, vnodes, hooks, nextSibling, ns) { + if (vnodes.length) { + validateKeys(vnodes, vnodes[0] != null && vnodes[0].key != null) + createNodesUnchecked(parent, vnodes, 0, vnodes.length, hooks, nextSibling, ns) + } + } + function createNodesUnchecked(parent, vnodes, start, end, hooks, nextSibling, ns) { for (var i = start; i < end; i++) { var vnode = vnodes[i] if (vnode != null) { @@ -99,7 +115,7 @@ module.exports = function($window) { var fragment = $doc.createDocumentFragment() if (vnode.children != null) { var children = vnode.children - createNodes(fragment, children, 0, children.length, hooks, null, ns) + createNodesChecked(fragment, children, hooks, null, ns) } vnode.dom = fragment.firstChild vnode.domSize = fragment.childNodes.length @@ -130,7 +146,7 @@ module.exports = function($window) { } if (vnode.children != null) { var children = vnode.children - createNodes(element, children, 0, children.length, hooks, null, ns) + createNodesChecked(element, children, hooks, null, ns) if (vnode.tag === "select" && attrs != null) setLateSelectAttrs(vnode, attrs) } } @@ -273,26 +289,19 @@ module.exports = function($window) { function updateNodes(parent, old, vnodes, hooks, nextSibling, ns) { if (old === vnodes || old == null && vnodes == null) return - else if (old == null || old.length === 0) createNodes(parent, vnodes, 0, vnodes.length, hooks, nextSibling, ns) + else if (old == null || old.length === 0) createNodesChecked(parent, vnodes, hooks, nextSibling, ns) else if (vnodes == null || vnodes.length === 0) removeNodes(old, 0, old.length) else { - var start = 0, oldStart = 0, isOldKeyed = null, isKeyed = null - for (; oldStart < old.length; oldStart++) { - if (old[oldStart] != null) { - isOldKeyed = old[oldStart].key != null - break - } - } - for (; start < vnodes.length; start++) { - if (vnodes[start] != null) { - isKeyed = vnodes[start].key != null - break - } - } + var isOldKeyed = old[0] != null && old[0].key != null + var isKeyed = vnodes[0] != null && vnodes[0].key != null + var start = 0, oldStart = 0 + validateKeys(vnodes, isKeyed) + if (!isOldKeyed) while (oldStart < old.length && old[oldStart] == null) oldStart++ + if (!isKeyed) while (start < vnodes.length && vnodes[start] == null) start++ if (isKeyed === null && isOldKeyed == null) return // both lists are full of nulls if (isOldKeyed !== isKeyed) { removeNodes(old, oldStart, old.length) - createNodes(parent, vnodes, start, vnodes.length, hooks, nextSibling, ns) + createNodesUnchecked(parent, vnodes, start, vnodes.length, hooks, nextSibling, ns) } else if (!isKeyed) { // Don't index past the end of either list (causes deopts). var commonLength = old.length < vnodes.length ? old.length : vnodes.length @@ -309,7 +318,7 @@ module.exports = function($window) { else updateNode(parent, o, v, hooks, getNextSibling(old, start + 1, nextSibling), ns) } if (old.length > commonLength) removeNodes(old, start, old.length) - if (vnodes.length > commonLength) createNodes(parent, vnodes, start, vnodes.length, hooks, nextSibling, ns) + if (vnodes.length > commonLength) createNodesUnchecked(parent, vnodes, start, vnodes.length, hooks, nextSibling, ns) } else { // keyed diff var oldEnd = old.length - 1, end = vnodes.length - 1, map, o, v, oe, ve, topSibling @@ -318,46 +327,30 @@ module.exports = function($window) { while (oldEnd >= oldStart && end >= start) { oe = old[oldEnd] ve = vnodes[end] - if (oe == null) oldEnd-- - else if (ve == null) end-- - else if (oe.key === ve.key) { - if (oe !== ve) updateNode(parent, oe, ve, hooks, nextSibling, ns) - if (ve.dom != null) nextSibling = ve.dom - oldEnd--, end-- - } else { - break - } + if (oe.key !== ve.key) break + if (oe !== ve) updateNode(parent, oe, ve, hooks, nextSibling, ns) + if (ve.dom != null) nextSibling = ve.dom + oldEnd--, end-- } // top-down while (oldEnd >= oldStart && end >= start) { o = old[oldStart] v = vnodes[start] - if (o == null) oldStart++ - else if (v == null) start++ - else if (o.key === v.key) { - oldStart++, start++ - if (o !== v) updateNode(parent, o, v, hooks, getNextSibling(old, oldStart, nextSibling), ns) - } else { - break - } + if (o.key !== v.key) break + oldStart++, start++ + if (o !== v) updateNode(parent, o, v, hooks, getNextSibling(old, oldStart, nextSibling), ns) } // swaps and list reversals while (oldEnd >= oldStart && end >= start) { - if (o == null) oldStart++ - else if (v == null) start++ - else if (oe == null) oldEnd-- - else if (ve == null) end-- - else if (start === end) break - else { - if (o.key !== ve.key || oe.key !== v.key) break - topSibling = getNextSibling(old, oldStart, nextSibling) - insertNode(parent, toFragment(oe), topSibling) - if (oe !== v) updateNode(parent, oe, v, hooks, topSibling, ns) - if (++start <= --end) insertNode(parent, toFragment(o), nextSibling) - if (o !== ve) updateNode(parent, o, ve, hooks, nextSibling, ns) - if (ve.dom != null) nextSibling = ve.dom - oldStart++; oldEnd-- - } + if (start === end) break + if (o.key !== ve.key || oe.key !== v.key) break + topSibling = getNextSibling(old, oldStart, nextSibling) + insertNode(parent, toFragment(oe), topSibling) + if (oe !== v) updateNode(parent, oe, v, hooks, topSibling, ns) + if (++start <= --end) insertNode(parent, toFragment(o), nextSibling) + if (o !== ve) updateNode(parent, o, ve, hooks, nextSibling, ns) + if (ve.dom != null) nextSibling = ve.dom + oldStart++; oldEnd-- oe = old[oldEnd] ve = vnodes[end] o = old[oldStart] @@ -365,20 +358,15 @@ module.exports = function($window) { } // bottom up once again while (oldEnd >= oldStart && end >= start) { - if (oe == null) oldEnd-- - else if (ve == null) end-- - else if (oe.key === ve.key) { - if (oe !== ve) updateNode(parent, oe, ve, hooks, nextSibling, ns) - if (ve.dom != null) nextSibling = ve.dom - oldEnd--, end-- - } else { - break - } + if (oe.key !== ve.key) break + if (oe !== ve) updateNode(parent, oe, ve, hooks, nextSibling, ns) + if (ve.dom != null) nextSibling = ve.dom + oldEnd--, end-- oe = old[oldEnd] ve = vnodes[end] } if (start > end) removeNodes(old, oldStart, oldEnd + 1) - else if (oldStart > oldEnd) createNodes(parent, vnodes, start, end + 1, hooks, nextSibling, ns) + else if (oldStart > oldEnd) createNodesUnchecked(parent, vnodes, start, end + 1, hooks, nextSibling, ns) else { // inspired by ivi https://github.com/ivijs/ivi/ by Boris Kaul var originalNextSibling = nextSibling, vnodesLength = end - start + 1, oldIndices = new Array(vnodesLength), li=0, i=0, pos = 2147483647, matched = 0, map, lisIndices @@ -386,22 +374,20 @@ module.exports = function($window) { for (i = end; i >= start; i--) { if (map == null) map = getKeyMap(old, oldStart, oldEnd + 1) ve = vnodes[i] - if (ve != null) { - var oldIndex = map[ve.key] - if (oldIndex != null) { - pos = (oldIndex < pos) ? oldIndex : -1 // becomes -1 if nodes were re-ordered - oldIndices[i-start] = oldIndex - oe = old[oldIndex] - old[oldIndex] = null - if (oe !== ve) updateNode(parent, oe, ve, hooks, nextSibling, ns) - if (ve.dom != null) nextSibling = ve.dom - matched++ - } + var oldIndex = map[ve.key] + if (oldIndex != null) { + pos = (oldIndex < pos) ? oldIndex : -1 // becomes -1 if nodes were re-ordered + oldIndices[i-start] = oldIndex + oe = old[oldIndex] + old[oldIndex] = null + if (oe !== ve) updateNode(parent, oe, ve, hooks, nextSibling, ns) + if (ve.dom != null) nextSibling = ve.dom + matched++ } } nextSibling = originalNextSibling if (matched !== oldEnd - oldStart + 1) removeNodes(old, oldStart, oldEnd + 1) - if (matched === 0) createNodes(parent, vnodes, start, end + 1, hooks, nextSibling, ns) + if (matched === 0) createNodesUnchecked(parent, vnodes, start, end + 1, hooks, nextSibling, ns) else { if (pos === -1) { // the indices of the indices of the items that are part of the @@ -541,26 +527,26 @@ module.exports = function($window) { // occur multiple times) and returns an array with the indices // of the items that are part of the longest increasing // subsequece + var lisTemp = [] function makeLisIndices(a) { - var p = a.slice() - var result = [] - result.push(0) - var u - var v - for (var i = 0, il = a.length; i < il; ++i) { - if (a[i] === -1) { - continue - } + var result = [0] + var u = 0, v = 0, i = 0 + var il = lisTemp.length = a.length + for (var i = 0; i < il; i++) lisTemp[i] = a[i] + for (var i = 0; i < il; ++i) { + if (a[i] === -1) continue var j = result[result.length - 1] if (a[j] < a[i]) { - p[i] = j + lisTemp[i] = j result.push(i) continue } u = 0 v = result.length - 1 while (u < v) { - var c = ((u + v) / 2) | 0 // eslint-disable-line no-bitwise + // Fast integer average without overflow. + // eslint-disable-next-line no-bitwise + var c = (u >>> 1) + (v >>> 1) + (u & v & 1) if (a[result[c]] < a[i]) { u = c + 1 } @@ -569,9 +555,7 @@ module.exports = function($window) { } } if (a[i] < a[result[u]]) { - if (u > 0) { - p[i] = result[u - 1] - } + if (u > 0) lisTemp[i] = result[u - 1] result[u] = i } } @@ -579,8 +563,9 @@ module.exports = function($window) { v = result[u - 1] while (u-- > 0) { result[u] = v - v = p[v] + v = lisTemp[v] } + lisTemp.length = 0 return result } diff --git a/render/tests/test-component.js b/render/tests/test-component.js index 29c04a44..9b2e90e2 100644 --- a/render/tests/test-component.js +++ b/render/tests/test-component.js @@ -110,7 +110,7 @@ o.spec("component", function() { visible = false render(root, [{tag: component}]) - o(root.firstChild.nodeValue).equals("") + o(root.childNodes.length).equals(0) }) o("updates root from null to null", function() { var component = createComponent({ @@ -218,7 +218,7 @@ o.spec("component", function() { o(root.firstChild.nodeType).equals(3) o(root.firstChild.nodeValue).equals("0") }) - o("can return boolean", function() { + o("can return `true`", function() { var component = createComponent({ view: function() { return true @@ -226,10 +226,9 @@ o.spec("component", function() { }) render(root, [{tag: component}]) - o(root.firstChild.nodeType).equals(3) - o(root.firstChild.nodeValue).equals("true") + o(root.childNodes.length).equals(0) }) - o("can return falsy boolean", function() { + o("can return `false`", function() { var component = createComponent({ view: function() { return false @@ -237,8 +236,7 @@ o.spec("component", function() { }) render(root, [{tag: component}]) - o(root.firstChild.nodeType).equals(3) - o(root.firstChild.nodeValue).equals("") + o(root.childNodes.length).equals(0) }) o("can return null", function() { var component = createComponent({ @@ -293,8 +291,7 @@ o.spec("component", function() { }) render(root, [{tag: component}]) - o(root.firstChild.nodeType).equals(3) - o(root.firstChild.nodeValue).equals("") + o(root.childNodes.length).equals(0) try { render(root, [{tag: component}]) diff --git a/render/tests/test-fragment.js b/render/tests/test-fragment.js index 528fa9a9..e6883161 100644 --- a/render/tests/test-fragment.js +++ b/render/tests/test-fragment.js @@ -57,25 +57,23 @@ function runTest(name, fragment) { var vnode = fragment([1]) o(vnode.children[0].tag).equals("#") - o(vnode.children[0].children).equals(1) + o(vnode.children[0].children).equals("1") }) o("handles falsy number single child", function() { var vnode = fragment([0]) o(vnode.children[0].tag).equals("#") - o(vnode.children[0].children).equals(0) + o(vnode.children[0].children).equals("0") }) o("handles boolean single child", function() { var vnode = fragment([true]) - o(vnode.children[0].tag).equals("#") - o(vnode.children[0].children).equals(true) + o(vnode.children).deepEquals([null]) }) o("handles falsy boolean single child", function() { var vnode = fragment([false]) - o(vnode.children[0].tag).equals("#") - o(vnode.children[0].children).equals("") + o(vnode.children).deepEquals([null]) }) o("handles null single child", function() { var vnode = fragment([null]) @@ -85,7 +83,7 @@ function runTest(name, fragment) { o("handles undefined single child", function() { var vnode = fragment([undefined]) - o(vnode.children[0]).equals(undefined) + o(vnode.children).deepEquals([null]) }) o("handles multiple string children", function() { var vnode = fragment(["", "a"]) @@ -99,29 +97,25 @@ function runTest(name, fragment) { var vnode = fragment([0, 1]) o(vnode.children[0].tag).equals("#") - o(vnode.children[0].children).equals(0) + o(vnode.children[0].children).equals("0") o(vnode.children[1].tag).equals("#") - o(vnode.children[1].children).equals(1) + o(vnode.children[1].children).equals("1") }) o("handles multiple boolean children", function() { var vnode = fragment([false, true]) - o(vnode.children[0].tag).equals("#") - o(vnode.children[0].children).equals("") - o(vnode.children[1].tag).equals("#") - o(vnode.children[1].children).equals(true) + o(vnode.children).deepEquals([null, null]) }) o("handles multiple null/undefined child", function() { var vnode = fragment([null, undefined]) - o(vnode.children[0]).equals(null) - o(vnode.children[1]).equals(undefined) + o(vnode.children).deepEquals([null, null]) }) o("handles falsy number single child without attrs", function() { var vnode = fragment(0) o(vnode.children[0].tag).equals("#") - o(vnode.children[0].children).equals(0) + o(vnode.children[0].children).equals("0") }) }) o.spec("children with attrs", function() { @@ -141,35 +135,33 @@ function runTest(name, fragment) { var vnode = fragment({}, [1]) o(vnode.children[0].tag).equals("#") - o(vnode.children[0].children).equals(1) + o(vnode.children[0].children).equals("1") }) o("handles falsy number single child", function() { var vnode = fragment({}, [0]) o(vnode.children[0].tag).equals("#") - o(vnode.children[0].children).equals(0) + o(vnode.children[0].children).equals("0") }) o("handles boolean single child", function() { var vnode = fragment({}, [true]) - o(vnode.children[0].tag).equals("#") - o(vnode.children[0].children).equals(true) + o(vnode.children).deepEquals([null]) }) o("handles falsy boolean single child", function() { var vnode = fragment({}, [false]) - o(vnode.children[0].tag).equals("#") - o(vnode.children[0].children).equals("") + o(vnode.children).deepEquals([null]) }) o("handles null single child", function() { var vnode = fragment({}, [null]) - o(vnode.children[0]).equals(null) + o(vnode.children).deepEquals([null]) }) o("handles undefined single child", function() { var vnode = fragment({}, [undefined]) - o(vnode.children[0]).equals(undefined) + o(vnode.children).deepEquals([null]) }) o("handles multiple string children", function() { var vnode = fragment({}, ["", "a"]) @@ -183,23 +175,19 @@ function runTest(name, fragment) { var vnode = fragment({}, [0, 1]) o(vnode.children[0].tag).equals("#") - o(vnode.children[0].children).equals(0) + o(vnode.children[0].children).equals("0") o(vnode.children[1].tag).equals("#") - o(vnode.children[1].children).equals(1) + o(vnode.children[1].children).equals("1") }) o("handles multiple boolean children", function() { var vnode = fragment({}, [false, true]) - o(vnode.children[0].tag).equals("#") - o(vnode.children[0].children).equals("") - o(vnode.children[1].tag).equals("#") - o(vnode.children[1].children).equals(true) + o(vnode.children).deepEquals([null, null]) }) o("handles multiple null/undefined child", function() { var vnode = fragment({}, [null, undefined]) - o(vnode.children[0]).equals(null) - o(vnode.children[1]).equals(undefined) + o(vnode.children).deepEquals([null, null]) }) }) }) diff --git a/render/tests/test-hyperscript.js b/render/tests/test-hyperscript.js index c2b6c98d..34d35974 100644 --- a/render/tests/test-hyperscript.js +++ b/render/tests/test-hyperscript.js @@ -382,32 +382,32 @@ o.spec("hyperscript", function() { o("handles number single child", function() { var vnode = m("div", {}, [1]) - o(vnode.text).equals(1) + o(vnode.text).equals("1") }) o("handles falsy number single child", function() { var vnode = m("div", {}, [0]) - o(vnode.text).equals(0) + o(vnode.text).equals("0") }) o("handles boolean single child", function() { var vnode = m("div", {}, [true]) - o(vnode.text).equals(true) + o(vnode.children).deepEquals([null]) }) o("handles falsy boolean single child", function() { var vnode = m("div", {}, [false]) - o(vnode.text).equals("") + o(vnode.children).deepEquals([null]) }) o("handles null single child", function() { var vnode = m("div", {}, [null]) - o(vnode.children[0]).equals(null) + o(vnode.children).deepEquals([null]) }) o("handles undefined single child", function() { var vnode = m("div", {}, [undefined]) - o(vnode.children[0]).equals(undefined) + o(vnode.children).deepEquals([null]) }) o("handles multiple string children", function() { var vnode = m("div", {}, ["", "a"]) @@ -421,28 +421,24 @@ o.spec("hyperscript", function() { var vnode = m("div", {}, [0, 1]) o(vnode.children[0].tag).equals("#") - o(vnode.children[0].children).equals(0) + o(vnode.children[0].children).equals("0") o(vnode.children[1].tag).equals("#") - o(vnode.children[1].children).equals(1) + o(vnode.children[1].children).equals("1") }) o("handles multiple boolean children", function() { var vnode = m("div", {}, [false, true]) - o(vnode.children[0].tag).equals("#") - o(vnode.children[0].children).equals("") - o(vnode.children[1].tag).equals("#") - o(vnode.children[1].children).equals(true) + o(vnode.children).deepEquals([null, null]) }) o("handles multiple null/undefined child", function() { var vnode = m("div", {}, [null, undefined]) - o(vnode.children[0]).equals(null) - o(vnode.children[1]).equals(undefined) + o(vnode.children).deepEquals([null, null]) }) o("handles falsy number single child without attrs", function() { var vnode = m("div", 0) - o(vnode.text).equals(0) + o(vnode.text).equals("0") }) }) o.spec("permutations", function() { @@ -511,31 +507,31 @@ o.spec("hyperscript", function() { var vnode = m("div", {a: "b"}, [1]) o(vnode.attrs.a).equals("b") - o(vnode.text).equals(1) + o(vnode.text).equals("1") }) o("handles attr and single falsy number text child", function() { var vnode = m("div", {a: "b"}, [0]) o(vnode.attrs.a).equals("b") - o(vnode.text).equals(0) + o(vnode.text).equals("0") }) o("handles attr and single boolean text child", function() { var vnode = m("div", {a: "b"}, [true]) o(vnode.attrs.a).equals("b") - o(vnode.text).equals(true) + o(vnode.children).deepEquals([null]) }) o("handles attr and single falsy boolean text child", function() { var vnode = m("div", {a: "b"}, [0]) o(vnode.attrs.a).equals("b") - o(vnode.text).equals(0) + o(vnode.text).equals("0") }) o("handles attr and single false boolean text child", function() { var vnode = m("div", {a: "b"}, [false]) o(vnode.attrs.a).equals("b") - o(vnode.text).equals("") + o(vnode.children).deepEquals([null]) }) o("handles attr and single text child unwrapped", function() { var vnode = m("div", {a: "b"}, "c") diff --git a/render/tests/test-normalize.js b/render/tests/test-normalize.js index 237612a4..2036d2ac 100644 --- a/render/tests/test-normalize.js +++ b/render/tests/test-normalize.js @@ -34,24 +34,22 @@ o.spec("normalize", function() { var node = Vnode.normalize(1) o(node.tag).equals("#") - o(node.children).equals(1) + o(node.children).equals("1") }) o("normalizes falsy number into text node", function() { var node = Vnode.normalize(0) o(node.tag).equals("#") - o(node.children).equals(0) + o(node.children).equals("0") }) - o("normalizes boolean into text node", function() { + o("normalizes `true` to `null`", function() { var node = Vnode.normalize(true) - o(node.tag).equals("#") - o(node.children).equals(true) + o(node).equals(null) }) - o("normalizes falsy boolean into empty text node", function() { + o("normalizes `false` to `null`", function() { var node = Vnode.normalize(false) - o(node.tag).equals("#") - o(node.children).equals("") + o(node).equals(null) }) }) diff --git a/render/tests/test-normalizeChildren.js b/render/tests/test-normalizeChildren.js index 699f0eab..cb1122c7 100644 --- a/render/tests/test-normalizeChildren.js +++ b/render/tests/test-normalizeChildren.js @@ -16,10 +16,9 @@ o.spec("normalizeChildren", function() { o(children[0].tag).equals("#") o(children[0].children).equals("a") }) - o("normalizes `false` values into empty string text nodes", function() { + o("normalizes `false` values into `null`s", function() { var children = Vnode.normalizeChildren([false]) - o(children[0].tag).equals("#") - o(children[0].children).equals("") + o(children[0]).equals(null) }) }) diff --git a/render/tests/test-updateNodes.js b/render/tests/test-updateNodes.js index 3fffaaf3..f6bf3668 100644 --- a/render/tests/test-updateNodes.js +++ b/render/tests/test-updateNodes.js @@ -451,9 +451,9 @@ o.spec("updateNodes", function() { o(updated[1].dom.nodeName).equals("I") o(updated[1].dom).equals(root.childNodes[2]) }) - o("populates array followed by null then el", function() { - var vnodes = [{tag: "[", key: 1, children: []}, null, {tag: "i", key: 2}] - var updated = [{tag: "[", key: 1, children: [{tag: "a"}, {tag: "b"}]}, null, {tag: "i", key: 2}] + o("populates array followed by el keyed", function() { + var vnodes = [{tag: "[", key: 1, children: []}, {tag: "i", key: 2}] + var updated = [{tag: "[", key: 1, children: [{tag: "a"}, {tag: "b"}]}, {tag: "i", key: 2}] render(root, vnodes) render(root, updated) @@ -464,10 +464,38 @@ o.spec("updateNodes", function() { o(updated[0].domSize).equals(2) o(updated[0].dom.nextSibling.nodeName).equals("B") o(updated[0].dom.nextSibling).equals(root.childNodes[1]) - o(updated[2].dom.nodeName).equals("I") - o(updated[2].dom).equals(root.childNodes[2]) + o(updated[1].dom.nodeName).equals("I") + o(updated[1].dom).equals(root.childNodes[2]) }) - o("populates childless array followed by el", function() { + o("throws populates array followed by el keyed", function() { + var vnodes = [{tag: "[", key: 1, children: []}, {tag: "i", key: 2}] + var updated = [{tag: "[", key: 1, children: [{tag: "a"}, {tag: "b"}]}, {tag: "i", key: 2}] + + render(root, vnodes) + render(root, updated) + + o(root.childNodes.length).equals(3) + o(updated[0].dom.nodeName).equals("A") + o(updated[0].dom).equals(root.childNodes[0]) + o(updated[0].domSize).equals(2) + o(updated[0].dom.nextSibling.nodeName).equals("B") + o(updated[0].dom.nextSibling).equals(root.childNodes[1]) + o(updated[1].dom.nodeName).equals("I") + o(updated[1].dom).equals(root.childNodes[2]) + }) + o("throws if array followed by null then el on first render keyed", function() { + var vnodes = [{tag: "[", key: 1, children: []}, null, {tag: "i", key: 2}] + + o(function () { render(root, vnodes) }).throws(TypeError) + }) + o("throws if array followed by null then el on next render keyed", function() { + var vnodes = [{tag: "[", key: 1, children: []}, {tag: "i", key: 2}] + var updated = [{tag: "[", key: 1, children: [{tag: "a"}, {tag: "b"}]}, null, {tag: "i", key: 2}] + + render(root, vnodes) + o(function () { render(root, updated) }).throws(TypeError) + }) + o("populates childless array replaced followed by el keyed", function() { var vnodes = [{tag: "[", key: 1}, {tag: "i", key: 2}] var updated = [{tag: "[", key: 1, children: [{tag: "a"}, {tag: "b"}]}, {tag: "i", key: 2}] @@ -483,21 +511,12 @@ o.spec("updateNodes", function() { o(updated[1].dom.nodeName).equals("I") o(updated[1].dom).equals(root.childNodes[2]) }) - o("populates childless array followed by null then el", function() { - var vnodes = [{tag: "[", key: 1}, null, {tag: "i", key: 2}] + o("throws if childless array replaced followed by null then el keyed", function() { + var vnodes = [{tag: "[", key: 1}, {tag: "i", key: 2}] var updated = [{tag: "[", key: 1, children: [{tag: "a"}, {tag: "b"}]}, null, {tag: "i", key: 2}] render(root, vnodes) - render(root, updated) - - o(root.childNodes.length).equals(3) - o(updated[0].dom.nodeName).equals("A") - o(updated[0].dom).equals(root.childNodes[0]) - o(updated[0].domSize).equals(2) - o(updated[0].dom.nextSibling.nodeName).equals("B") - o(updated[0].dom.nextSibling).equals(root.childNodes[1]) - o(updated[2].dom.nodeName).equals("I") - o(updated[2].dom).equals(root.childNodes[2]) + o(function () { render(root, updated) }).throws(TypeError) }) o("moves from end to start", function() { var vnodes = [{tag: "a", key: 1}, {tag: "b", key: 2}, {tag: "i", key: 3}, {tag: "s", key: 4}] diff --git a/render/vnode.js b/render/vnode.js index 5ccc0b27..11560811 100644 --- a/render/vnode.js +++ b/render/vnode.js @@ -5,8 +5,9 @@ function Vnode(tag, key, attrs, children, text, dom) { } Vnode.normalize = function(node) { if (Array.isArray(node)) return Vnode("[", undefined, undefined, Vnode.normalizeChildren(node), undefined, undefined) - if (node != null && typeof node !== "object") return Vnode("#", undefined, undefined, node === false ? "" : node, undefined, undefined) - return node + if (node == null || typeof node === "boolean") return null + if (typeof node === "object") return node + return Vnode("#", undefined, undefined, String(node), undefined, undefined) } Vnode.normalizeChildren = function(input) { var children = []