From 665578060e9d560c85494a05efc88650d5be3290 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Date: Thu, 9 Jun 2022 15:12:08 +0200 Subject: [PATCH] Address review comments, linter and build concerns --- render/dom-for.js | 3 +- render/render.js | 46 ++--- render/tests/test-dom-for.js | 330 +++++++++++++++++----------------- render/tests/test-onremove.js | 51 +++--- test-utils/domMock.js | 2 +- 5 files changed, 224 insertions(+), 208 deletions(-) diff --git a/render/dom-for.js b/render/dom-for.js index 11c252c4..7cf33a4b 100644 --- a/render/dom-for.js +++ b/render/dom-for.js @@ -1,6 +1,7 @@ "use strict" -var delayedRemoval = module.exports.delayedRemoval = new WeakMap +var delayedRemoval = new WeakMap +module.exports.delayedRemoval = delayedRemoval module.exports.domFor = function *domFor(vnode, {generation} = {generation: undefined}) { let {dom, domSize} = vnode diff --git a/render/render.js b/render/render.js index 8d0ea0a7..6d267606 100644 --- a/render/render.js +++ b/render/render.js @@ -1,19 +1,21 @@ "use strict" -const Vnode = require("../render/vnode") -const {domFor, delayedRemoval} = require("../render/dom-for") +var Vnode = require("../render/vnode") +var df = require("../render/dom-for") +var delayedRemoval = df.delayedRemoval +var domFor = df.domFor module.exports = function($window) { - const $doc = $window && $window.document + var $doc = $window && $window.document - const nameSpace = { + var nameSpace = { svg: "http://www.w3.org/2000/svg", math: "http://www.w3.org/1998/Math/MathML" } - let currentRedraw - let currentDOM - let currentRender + var currentRedraw + var currentDOM + var currentRender function getNameSpace(vnode) { return vnode.attrs && vnode.attrs.xmlns || nameSpace[vnode.tag] @@ -71,7 +73,7 @@ module.exports = function($window) { } function createText(parent, vnode, nextSibling) { vnode.dom = $doc.createTextNode(vnode.children) - insertNode(parent, vnode.dom, nextSibling) + insertDOM(parent, vnode.dom, nextSibling) } var possibleParents = {caption: "table", thead: "table", tbody: "table", tfoot: "table", tr: "tbody", th: "tr", td: "tr", colgroup: "table", col: "colgroup"} function createHTML(parent, vnode, ns, nextSibling) { @@ -96,7 +98,7 @@ module.exports = function($window) { while (child = temp.firstChild) { fragment.appendChild(child) } - insertNode(parent, fragment, nextSibling) + insertDOM(parent, fragment, nextSibling) } function createFragment(parent, vnode, hooks, ns, nextSibling) { var fragment = $doc.createDocumentFragment() @@ -106,7 +108,7 @@ module.exports = function($window) { } vnode.dom = fragment.firstChild vnode.domSize = fragment.childNodes.length - insertNode(parent, fragment, nextSibling) + insertDOM(parent, fragment, nextSibling) } function createElement(parent, vnode, hooks, ns, nextSibling) { var tag = vnode.tag @@ -124,7 +126,7 @@ module.exports = function($window) { setAttrs(vnode, attrs, ns) } - insertNode(parent, element, nextSibling) + insertDOM(parent, element, nextSibling) if (!maybeSetContentEditable(vnode)) { if (vnode.children != null) { @@ -321,9 +323,9 @@ module.exports = function($window) { if (start === end) break if (o.key !== ve.key || oe.key !== v.key) break topSibling = getNextSibling(old, oldStart, nextSibling) - moveNodes(parent, oe, topSibling) + moveNode(parent, oe, topSibling) if (oe !== v) updateNode(parent, oe, v, hooks, topSibling, ns) - if (++start <= --end) moveNodes(parent, o, nextSibling) + if (++start <= --end) moveNode(parent, o, nextSibling) if (o !== ve) updateNode(parent, o, ve, hooks, nextSibling, ns) if (ve.dom != null) nextSibling = ve.dom oldStart++; oldEnd-- @@ -375,7 +377,7 @@ module.exports = function($window) { if (oldIndices[i-start] === -1) createNode(parent, v, hooks, ns, nextSibling) else { if (lisIndices[li] === i - start) li-- - else moveNodes(parent, v, nextSibling) + else moveNode(parent, v, nextSibling) } if (v.dom != null) nextSibling = vnodes[i].dom } @@ -544,8 +546,8 @@ module.exports = function($window) { return nextSibling } - // This handles fragments with zombie children (removed from vdom, but persisted in DOM throug onbeforeremove) - function moveNodes(parent, vnode, nextSibling) { + // This handles fragments with zombie children (removed from vdom, but persisted in DOM through onbeforeremove) + function moveNode(parent, vnode, nextSibling) { if (vnode.dom != null) { var target if (vnode.domSize == null) { @@ -553,13 +555,13 @@ module.exports = function($window) { target = vnode.dom } else { target = $doc.createDocumentFragment() - for (const dom of domFor(vnode)) target.appendChild(dom) + for (var dom of domFor(vnode)) target.appendChild(dom) } - insertNode(parent, target, nextSibling) + insertDOM(parent, target, nextSibling) } } - function insertNode(parent, dom, nextSibling) { + function insertDOM(parent, dom, nextSibling) { if (nextSibling != null) parent.insertBefore(dom, nextSibling) else parent.appendChild(dom) } @@ -612,8 +614,8 @@ module.exports = function($window) { removeDOM(parent, vnode, undefined) } else { generation = currentRender - for (const dom of domFor(vnode)) delayedRemoval.set(dom, generation) - function finalizer(a, b) { + for (var dom of domFor(vnode)) delayedRemoval.set(dom, generation) + var finalizer = function(a, b) { return function () { // eslint-disable-next-line no-bitwise if (mask & a) { mask &= b; if (!mask) { @@ -637,7 +639,7 @@ module.exports = function($window) { // don't allocate for the common case if (delayedRemoval.get(vnode.dom) === generation) parent.removeChild(vnode.dom) } else { - for (const dom of domFor(vnode, {generation})) parent.removeChild(dom) + for (var dom of domFor(vnode, {generation})) parent.removeChild(dom) } } diff --git a/render/tests/test-dom-for.js b/render/tests/test-dom-for.js index d36df4a2..8ea7c332 100644 --- a/render/tests/test-dom-for.js +++ b/render/tests/test-dom-for.js @@ -1,176 +1,178 @@ -var o = require("ospec") -var components = require("../../test-utils/components") -var domMock = require("../../test-utils/domMock") -var vdom = require("../../render/render") -var m = require("../../render/hyperscript") -var fragment = require("../../render/fragment") -var domFor = require('../../render/dom-for').domFor +"use strict" + +const o = require("ospec") +const components = require("../../test-utils/components") +const domMock = require("../../test-utils/domMock") +const vdom = require("../../render/render") +const m = require("../../render/hyperscript") +const fragment = require("../../render/fragment") +const domFor = require("../../render/dom-for").domFor o.spec("domFor(vnode)", function() { - var $window, root, render + let $window, root, render o.beforeEach(function() { $window = domMock() root = $window.document.createElement("div") render = vdom($window) }) - o('works for simple vnodes', function() { - render(root, m('div', {oncreate(vnode){ - let n = 0 - for (const dom of domFor(vnode)) { - o(dom).equals(root.firstChild) - o(++n).equals(1) - } - }})) - }) - o('works for fragments', function () { - render(root, fragment({ - oncreate(vnode){ - let n = 0 - for (const dom of domFor(vnode)) { - o(dom).equals(root.childNodes[n]) - n++ - } - o(n).equals(2) - } - }, [ - m('a'), - m('b') - ])) - }) - o('works in fragments with children that have delayed removal', function() { - function oncreate(vnode){ - o(root.childNodes.length).equals(3) - o(root.childNodes[0].nodeName).equals('A') - o(root.childNodes[1].nodeName).equals('B') - o(root.childNodes[2].nodeName).equals('C') + o("works for simple vnodes", function() { + render(root, m("div", {oncreate(vnode){ + let n = 0 + for (const dom of domFor(vnode)) { + o(dom).equals(root.firstChild) + o(++n).equals(1) + } + }})) + }) + o("works for fragments", function () { + render(root, fragment({ + oncreate(vnode){ + let n = 0 + for (const dom of domFor(vnode)) { + o(dom).equals(root.childNodes[n]) + n++ + } + o(n).equals(2) + } + }, [ + m("a"), + m("b") + ])) + }) + o("works in fragments with children that have delayed removal", function() { + function oncreate(vnode){ + o(root.childNodes.length).equals(3) + o(root.childNodes[0].nodeName).equals("A") + o(root.childNodes[1].nodeName).equals("B") + o(root.childNodes[2].nodeName).equals("C") - const iter = domFor(vnode) - o(iter.next()).deepEquals({done:false, value: root.childNodes[0]}) - o(iter.next()).deepEquals({done:false, value: root.childNodes[1]}) - o(iter.next()).deepEquals({done:false, value: root.childNodes[2]}) - o(iter.next().done).deepEquals(true) - o(root.childNodes.length).equals(3) - } - function onupdate(vnode) { - // the b node is still present in the DOM - o(root.childNodes.length).equals(3) - o(root.childNodes[0].nodeName).equals('A') - o(root.childNodes[1].nodeName).equals('B') - o(root.childNodes[2].nodeName).equals('C') + const iter = domFor(vnode) + o(iter.next()).deepEquals({done:false, value: root.childNodes[0]}) + o(iter.next()).deepEquals({done:false, value: root.childNodes[1]}) + o(iter.next()).deepEquals({done:false, value: root.childNodes[2]}) + o(iter.next().done).deepEquals(true) + o(root.childNodes.length).equals(3) + } + function onupdate(vnode) { + // the b node is still present in the DOM + o(root.childNodes.length).equals(3) + o(root.childNodes[0].nodeName).equals("A") + o(root.childNodes[1].nodeName).equals("B") + o(root.childNodes[2].nodeName).equals("C") - const iter = domFor(vnode) - o(iter.next()).deepEquals({done:false, value: root.childNodes[0]}) - o(iter.next()).deepEquals({done:false, value: root.childNodes[2]}) - o(iter.next().done).deepEquals(true) - o(root.childNodes.length).equals(3) - } - - render(root, fragment( - {oncreate, onupdate}, - [ - m('a'), - m('b', {onbeforeremove(){return {then(){}, finally(){}}}}), - m('c') - ] - )) - render(root, fragment( - {oncreate, onupdate}, - [ - m('a'), - null, - m('c'), - ] - )) + const iter = domFor(vnode) + o(iter.next()).deepEquals({done:false, value: root.childNodes[0]}) + o(iter.next()).deepEquals({done:false, value: root.childNodes[2]}) + o(iter.next().done).deepEquals(true) + o(root.childNodes.length).equals(3) + } - }) - components.forEach(function(cmp){ - o.spec(cmp.kind, function(){ - var createComponent = cmp.create - o('works for components that return one element', function() { - const C = createComponent({ - view(){return m('div')}, - oncreate(vnode){ - let n = 0 - for (const dom of domFor(vnode)) { - o(dom).equals(root.firstChild) - o(++n).equals(1) - } - } - }) - render(root, m(C)) - }) - o('works for components that return fragments', function () { - const oncreate = o.spy(function oncreate(vnode){ - o(root.childNodes.length).equals(3) - o(root.childNodes[0].nodeName).equals('A') - o(root.childNodes[1].nodeName).equals('B') - o(root.childNodes[2].nodeName).equals('C') - - const iter = domFor(vnode) - o(iter.next()).deepEquals({done:false, value: root.childNodes[0]}) - o(iter.next()).deepEquals({done:false, value: root.childNodes[1]}) - o(iter.next()).deepEquals({done:false, value: root.childNodes[2]}) - o(iter.next().done).deepEquals(true) - o(root.childNodes.length).equals(3) - }) - const C = createComponent({ - view({children}){return children}, - oncreate - }) - render(root, m(C, [ - m('a'), - m('b'), - m('c') - ])) - o(oncreate.callCount).equals(1) - }) - o('works for components that return fragments with delayed removal', function () { - const onbeforeremove = o.spy(function onbeforeremove(){return {then(){}, finally(){}}}) - const oncreate = o.spy(function oncreate(vnode){ - o(root.childNodes.length).equals(3) - o(root.childNodes[0].nodeName).equals('A') - o(root.childNodes[1].nodeName).equals('B') - o(root.childNodes[2].nodeName).equals('C') - - const iter = domFor(vnode) - o(iter.next()).deepEquals({done:false, value: root.childNodes[0]}) - o(iter.next()).deepEquals({done:false, value: root.childNodes[1]}) - o(iter.next()).deepEquals({done:false, value: root.childNodes[2]}) - o(iter.next().done).deepEquals(true) - o(root.childNodes.length).equals(3) - }) - const onupdate = o.spy(function onupdate(vnode) { - o(root.childNodes.length).equals(3) - o(root.childNodes[0].nodeName).equals('A') - o(root.childNodes[1].nodeName).equals('B') - o(root.childNodes[2].nodeName).equals('C') - - const iter = domFor(vnode) + render(root, fragment( + {oncreate, onupdate}, + [ + m("a"), + m("b", {onbeforeremove(){return {then(){}, finally(){}}}}), + m("c") + ] + )) + render(root, fragment( + {oncreate, onupdate}, + [ + m("a"), + null, + m("c"), + ] + )) - o(iter.next()).deepEquals({done:false, value: root.childNodes[0]}) - o(iter.next()).deepEquals({done:false, value: root.childNodes[2]}) - o(iter.next().done).deepEquals(true) - o(root.childNodes.length).equals(3) - }) - const C = createComponent({ - view({children}){return children}, - oncreate, - onupdate - }) - render(root, m(C, [ - m('a'), - m('b', {onbeforeremove}), - m('c') - ])) - render(root, m(C, [ - m('a'), - null, - m('c') - ])) - o(oncreate.callCount).equals(1) - o(onupdate.callCount).equals(1) - o(onbeforeremove.callCount).equals(1) - }) - }) - }) + }) + components.forEach(function(cmp){ + const {kind, create: createComponent} = cmp + o.spec(kind, function(){ + o("works for components that return one element", function() { + const C = createComponent({ + view(){return m("div")}, + oncreate(vnode){ + let n = 0 + for (const dom of domFor(vnode)) { + o(dom).equals(root.firstChild) + o(++n).equals(1) + } + } + }) + render(root, m(C)) + }) + o("works for components that return fragments", function () { + const oncreate = o.spy(function oncreate(vnode){ + o(root.childNodes.length).equals(3) + o(root.childNodes[0].nodeName).equals("A") + o(root.childNodes[1].nodeName).equals("B") + o(root.childNodes[2].nodeName).equals("C") + + const iter = domFor(vnode) + o(iter.next()).deepEquals({done:false, value: root.childNodes[0]}) + o(iter.next()).deepEquals({done:false, value: root.childNodes[1]}) + o(iter.next()).deepEquals({done:false, value: root.childNodes[2]}) + o(iter.next().done).deepEquals(true) + o(root.childNodes.length).equals(3) + }) + const C = createComponent({ + view({children}){return children}, + oncreate + }) + render(root, m(C, [ + m("a"), + m("b"), + m("c") + ])) + o(oncreate.callCount).equals(1) + }) + o("works for components that return fragments with delayed removal", function () { + const onbeforeremove = o.spy(function onbeforeremove(){return {then(){}, finally(){}}}) + const oncreate = o.spy(function oncreate(vnode){ + o(root.childNodes.length).equals(3) + o(root.childNodes[0].nodeName).equals("A") + o(root.childNodes[1].nodeName).equals("B") + o(root.childNodes[2].nodeName).equals("C") + + const iter = domFor(vnode) + o(iter.next()).deepEquals({done:false, value: root.childNodes[0]}) + o(iter.next()).deepEquals({done:false, value: root.childNodes[1]}) + o(iter.next()).deepEquals({done:false, value: root.childNodes[2]}) + o(iter.next().done).deepEquals(true) + o(root.childNodes.length).equals(3) + }) + const onupdate = o.spy(function onupdate(vnode) { + o(root.childNodes.length).equals(3) + o(root.childNodes[0].nodeName).equals("A") + o(root.childNodes[1].nodeName).equals("B") + o(root.childNodes[2].nodeName).equals("C") + + const iter = domFor(vnode) + + o(iter.next()).deepEquals({done:false, value: root.childNodes[0]}) + o(iter.next()).deepEquals({done:false, value: root.childNodes[2]}) + o(iter.next().done).deepEquals(true) + o(root.childNodes.length).equals(3) + }) + const C = createComponent({ + view({children}){return children}, + oncreate, + onupdate + }) + render(root, m(C, [ + m("a"), + m("b", {onbeforeremove}), + m("c") + ])) + render(root, m(C, [ + m("a"), + null, + m("c") + ])) + o(oncreate.callCount).equals(1) + o(onupdate.callCount).equals(1) + o(onbeforeremove.callCount).equals(1) + }) + }) + }) }) \ No newline at end of file diff --git a/render/tests/test-onremove.js b/render/tests/test-onremove.js index 68331728..2a1d36ac 100644 --- a/render/tests/test-onremove.js +++ b/render/tests/test-onremove.js @@ -222,8 +222,8 @@ o.spec("onremove", function() { text: textNode.nodeValue, }) } - actual = JSON.stringify(actual, null, ' ') - expected = JSON.stringify(expected, null, ' ') + actual = JSON.stringify(actual, null, " ") + expected = JSON.stringify(expected, null, " ") return { pass: actual === expected, message: @@ -232,21 +232,24 @@ o.spec("onremove", function() { ${actual}` } }} - - var finallyCB + var finallyCB1 + var finallyCB2 + var C = createComponent({ + view({children}){return children}, + onbeforeremove(){ + return {then(){}, finally: function (fcb) { finallyCB1 = fcb }} + } + }) function update(id, showParent, showChild) { render(root, m("div", showParent && fragment( "", // Required - showChild && fragment( - { - onbeforeremove: function () { - return {then(){}, finally: function (fcb) { finallyCB = fcb }} - }, + showChild && m(C, { + onbeforeremove: function () { + return {then(){}, finally: function (fcb) { finallyCB2 = fcb }} }, - m("div", id) - ) + }, m("div", id)) ) ) ) @@ -257,7 +260,8 @@ ${actual}` ["#text", ""], ["DIV", "1"], ])) - o(finallyCB).equals(undefined) + o(finallyCB1).equals(undefined) + o(finallyCB2).equals(undefined) update("2", true, false) @@ -265,9 +269,10 @@ ${actual}` ["#text", ""], ["DIV", "1"], ])) - o(typeof finallyCB).equals("function") + o(typeof finallyCB1).equals("function") - var original = finallyCB + var original1 = finallyCB1 + var original2 = finallyCB2 update("3", true, true) @@ -276,14 +281,16 @@ ${actual}` ["DIV", "1"], ["DIV", "3"], ])) - o(finallyCB).equals(original) + o(finallyCB1).equals(original1) + o(finallyCB2).equals(original2) update("4", false, true) o(root).satisfies(template([ ["DIV", "1"], ])) - o(finallyCB).equals(original) + o(finallyCB1).equals(original1) + o(finallyCB2).equals(original2) update("5", true, true) @@ -292,22 +299,26 @@ ${actual}` ["#text", ""], ["DIV", "5"], ])) - o(finallyCB).equals(original) + o(finallyCB1).equals(original1) + o(finallyCB2).equals(original2) - finallyCB() + finallyCB1() + finallyCB2() o(root).satisfies(template([ ["#text", ""], ["DIV", "5"], ])) - o(finallyCB).equals(original) + o(finallyCB1).equals(original1) + o(finallyCB2).equals(original2) update("6", true, true) o(root).satisfies(template([ ["#text", ""], ["DIV", "6"], ])) - o(finallyCB).equals(original) + o(finallyCB1).equals(original1) + o(finallyCB2).equals(original2) }) }) }) diff --git a/test-utils/domMock.js b/test-utils/domMock.js index 18c839cd..f645595e 100644 --- a/test-utils/domMock.js +++ b/test-utils/domMock.js @@ -99,7 +99,7 @@ module.exports = function(options) { } } function removeChild(child) { - if (child == null || typeof child !== 'object' || !("nodeType" in child)) { + if (child == null || typeof child !== "object" || !("nodeType" in child)) { throw new TypeError("Failed to execute removeChild, parameter is not of type 'Node'") } var index = this.childNodes.indexOf(child)