Address review comments, linter and build concerns

This commit is contained in:
Pierre-Yves 2022-06-09 15:12:08 +02:00 committed by Pierre-Yves Gérardy
parent 3fd82e6359
commit 665578060e
5 changed files with 224 additions and 208 deletions

View file

@ -1,6 +1,7 @@
"use strict" "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}) { module.exports.domFor = function *domFor(vnode, {generation} = {generation: undefined}) {
let {dom, domSize} = vnode let {dom, domSize} = vnode

View file

@ -1,19 +1,21 @@
"use strict" "use strict"
const Vnode = require("../render/vnode") var Vnode = require("../render/vnode")
const {domFor, delayedRemoval} = require("../render/dom-for") var df = require("../render/dom-for")
var delayedRemoval = df.delayedRemoval
var domFor = df.domFor
module.exports = function($window) { 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", svg: "http://www.w3.org/2000/svg",
math: "http://www.w3.org/1998/Math/MathML" math: "http://www.w3.org/1998/Math/MathML"
} }
let currentRedraw var currentRedraw
let currentDOM var currentDOM
let currentRender var currentRender
function getNameSpace(vnode) { function getNameSpace(vnode) {
return vnode.attrs && vnode.attrs.xmlns || nameSpace[vnode.tag] return vnode.attrs && vnode.attrs.xmlns || nameSpace[vnode.tag]
@ -71,7 +73,7 @@ module.exports = function($window) {
} }
function createText(parent, vnode, nextSibling) { function createText(parent, vnode, nextSibling) {
vnode.dom = $doc.createTextNode(vnode.children) 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"} 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) { function createHTML(parent, vnode, ns, nextSibling) {
@ -96,7 +98,7 @@ module.exports = function($window) {
while (child = temp.firstChild) { while (child = temp.firstChild) {
fragment.appendChild(child) fragment.appendChild(child)
} }
insertNode(parent, fragment, nextSibling) insertDOM(parent, fragment, nextSibling)
} }
function createFragment(parent, vnode, hooks, ns, nextSibling) { function createFragment(parent, vnode, hooks, ns, nextSibling) {
var fragment = $doc.createDocumentFragment() var fragment = $doc.createDocumentFragment()
@ -106,7 +108,7 @@ module.exports = function($window) {
} }
vnode.dom = fragment.firstChild vnode.dom = fragment.firstChild
vnode.domSize = fragment.childNodes.length vnode.domSize = fragment.childNodes.length
insertNode(parent, fragment, nextSibling) insertDOM(parent, fragment, nextSibling)
} }
function createElement(parent, vnode, hooks, ns, nextSibling) { function createElement(parent, vnode, hooks, ns, nextSibling) {
var tag = vnode.tag var tag = vnode.tag
@ -124,7 +126,7 @@ module.exports = function($window) {
setAttrs(vnode, attrs, ns) setAttrs(vnode, attrs, ns)
} }
insertNode(parent, element, nextSibling) insertDOM(parent, element, nextSibling)
if (!maybeSetContentEditable(vnode)) { if (!maybeSetContentEditable(vnode)) {
if (vnode.children != null) { if (vnode.children != null) {
@ -321,9 +323,9 @@ module.exports = function($window) {
if (start === end) break if (start === end) break
if (o.key !== ve.key || oe.key !== v.key) break if (o.key !== ve.key || oe.key !== v.key) break
topSibling = getNextSibling(old, oldStart, nextSibling) topSibling = getNextSibling(old, oldStart, nextSibling)
moveNodes(parent, oe, topSibling) moveNode(parent, oe, topSibling)
if (oe !== v) updateNode(parent, oe, v, hooks, topSibling, ns) 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 (o !== ve) updateNode(parent, o, ve, hooks, nextSibling, ns)
if (ve.dom != null) nextSibling = ve.dom if (ve.dom != null) nextSibling = ve.dom
oldStart++; oldEnd-- oldStart++; oldEnd--
@ -375,7 +377,7 @@ module.exports = function($window) {
if (oldIndices[i-start] === -1) createNode(parent, v, hooks, ns, nextSibling) if (oldIndices[i-start] === -1) createNode(parent, v, hooks, ns, nextSibling)
else { else {
if (lisIndices[li] === i - start) li-- if (lisIndices[li] === i - start) li--
else moveNodes(parent, v, nextSibling) else moveNode(parent, v, nextSibling)
} }
if (v.dom != null) nextSibling = vnodes[i].dom if (v.dom != null) nextSibling = vnodes[i].dom
} }
@ -544,8 +546,8 @@ module.exports = function($window) {
return nextSibling return nextSibling
} }
// This handles fragments with zombie children (removed from vdom, but persisted in DOM throug onbeforeremove) // This handles fragments with zombie children (removed from vdom, but persisted in DOM through onbeforeremove)
function moveNodes(parent, vnode, nextSibling) { function moveNode(parent, vnode, nextSibling) {
if (vnode.dom != null) { if (vnode.dom != null) {
var target var target
if (vnode.domSize == null) { if (vnode.domSize == null) {
@ -553,13 +555,13 @@ module.exports = function($window) {
target = vnode.dom target = vnode.dom
} else { } else {
target = $doc.createDocumentFragment() 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) if (nextSibling != null) parent.insertBefore(dom, nextSibling)
else parent.appendChild(dom) else parent.appendChild(dom)
} }
@ -612,8 +614,8 @@ module.exports = function($window) {
removeDOM(parent, vnode, undefined) removeDOM(parent, vnode, undefined)
} else { } else {
generation = currentRender generation = currentRender
for (const dom of domFor(vnode)) delayedRemoval.set(dom, generation) for (var dom of domFor(vnode)) delayedRemoval.set(dom, generation)
function finalizer(a, b) { var finalizer = function(a, b) {
return function () { return function () {
// eslint-disable-next-line no-bitwise // eslint-disable-next-line no-bitwise
if (mask & a) { mask &= b; if (!mask) { if (mask & a) { mask &= b; if (!mask) {
@ -637,7 +639,7 @@ module.exports = function($window) {
// don't allocate for the common case // don't allocate for the common case
if (delayedRemoval.get(vnode.dom) === generation) parent.removeChild(vnode.dom) if (delayedRemoval.get(vnode.dom) === generation) parent.removeChild(vnode.dom)
} else { } else {
for (const dom of domFor(vnode, {generation})) parent.removeChild(dom) for (var dom of domFor(vnode, {generation})) parent.removeChild(dom)
} }
} }

View file

@ -1,176 +1,178 @@
var o = require("ospec") "use strict"
var components = require("../../test-utils/components")
var domMock = require("../../test-utils/domMock") const o = require("ospec")
var vdom = require("../../render/render") const components = require("../../test-utils/components")
var m = require("../../render/hyperscript") const domMock = require("../../test-utils/domMock")
var fragment = require("../../render/fragment") const vdom = require("../../render/render")
var domFor = require('../../render/dom-for').domFor const m = require("../../render/hyperscript")
const fragment = require("../../render/fragment")
const domFor = require("../../render/dom-for").domFor
o.spec("domFor(vnode)", function() { o.spec("domFor(vnode)", function() {
var $window, root, render let $window, root, render
o.beforeEach(function() { o.beforeEach(function() {
$window = domMock() $window = domMock()
root = $window.document.createElement("div") root = $window.document.createElement("div")
render = vdom($window) render = vdom($window)
}) })
o('works for simple vnodes', function() { o("works for simple vnodes", function() {
render(root, m('div', {oncreate(vnode){ render(root, m("div", {oncreate(vnode){
let n = 0 let n = 0
for (const dom of domFor(vnode)) { for (const dom of domFor(vnode)) {
o(dom).equals(root.firstChild) o(dom).equals(root.firstChild)
o(++n).equals(1) o(++n).equals(1)
} }
}})) }}))
}) })
o('works for fragments', function () { o("works for fragments", function () {
render(root, fragment({ render(root, fragment({
oncreate(vnode){ oncreate(vnode){
let n = 0 let n = 0
for (const dom of domFor(vnode)) { for (const dom of domFor(vnode)) {
o(dom).equals(root.childNodes[n]) o(dom).equals(root.childNodes[n])
n++ n++
} }
o(n).equals(2) o(n).equals(2)
} }
}, [ }, [
m('a'), m("a"),
m('b') m("b")
])) ]))
}) })
o('works in fragments with children that have delayed removal', function() { o("works in fragments with children that have delayed removal", function() {
function oncreate(vnode){ function oncreate(vnode){
o(root.childNodes.length).equals(3) o(root.childNodes.length).equals(3)
o(root.childNodes[0].nodeName).equals('A') o(root.childNodes[0].nodeName).equals("A")
o(root.childNodes[1].nodeName).equals('B') o(root.childNodes[1].nodeName).equals("B")
o(root.childNodes[2].nodeName).equals('C') o(root.childNodes[2].nodeName).equals("C")
const iter = domFor(vnode) const iter = domFor(vnode)
o(iter.next()).deepEquals({done:false, value: root.childNodes[0]}) 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[1]})
o(iter.next()).deepEquals({done:false, value: root.childNodes[2]}) o(iter.next()).deepEquals({done:false, value: root.childNodes[2]})
o(iter.next().done).deepEquals(true) o(iter.next().done).deepEquals(true)
o(root.childNodes.length).equals(3) o(root.childNodes.length).equals(3)
} }
function onupdate(vnode) { function onupdate(vnode) {
// the b node is still present in the DOM // the b node is still present in the DOM
o(root.childNodes.length).equals(3) o(root.childNodes.length).equals(3)
o(root.childNodes[0].nodeName).equals('A') o(root.childNodes[0].nodeName).equals("A")
o(root.childNodes[1].nodeName).equals('B') o(root.childNodes[1].nodeName).equals("B")
o(root.childNodes[2].nodeName).equals('C') o(root.childNodes[2].nodeName).equals("C")
const iter = domFor(vnode) const iter = domFor(vnode)
o(iter.next()).deepEquals({done:false, value: root.childNodes[0]}) o(iter.next()).deepEquals({done:false, value: root.childNodes[0]})
o(iter.next()).deepEquals({done:false, value: root.childNodes[2]}) o(iter.next()).deepEquals({done:false, value: root.childNodes[2]})
o(iter.next().done).deepEquals(true) o(iter.next().done).deepEquals(true)
o(root.childNodes.length).equals(3) 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'),
]
))
}) render(root, fragment(
components.forEach(function(cmp){ {oncreate, onupdate},
o.spec(cmp.kind, function(){ [
var createComponent = cmp.create m("a"),
o('works for components that return one element', function() { m("b", {onbeforeremove(){return {then(){}, finally(){}}}}),
const C = createComponent({ m("c")
view(){return m('div')}, ]
oncreate(vnode){ ))
let n = 0 render(root, fragment(
for (const dom of domFor(vnode)) { {oncreate, onupdate},
o(dom).equals(root.firstChild) [
o(++n).equals(1) m("a"),
} null,
} m("c"),
}) ]
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]}) components.forEach(function(cmp){
o(iter.next().done).deepEquals(true) const {kind, create: createComponent} = cmp
o(root.childNodes.length).equals(3) o.spec(kind, function(){
}) o("works for components that return one element", function() {
const C = createComponent({ const C = createComponent({
view({children}){return children}, view(){return m("div")},
oncreate, oncreate(vnode){
onupdate let n = 0
}) for (const dom of domFor(vnode)) {
render(root, m(C, [ o(dom).equals(root.firstChild)
m('a'), o(++n).equals(1)
m('b', {onbeforeremove}), }
m('c') }
])) })
render(root, m(C, [ render(root, m(C))
m('a'), })
null, o("works for components that return fragments", function () {
m('c') const oncreate = o.spy(function oncreate(vnode){
])) o(root.childNodes.length).equals(3)
o(oncreate.callCount).equals(1) o(root.childNodes[0].nodeName).equals("A")
o(onupdate.callCount).equals(1) o(root.childNodes[1].nodeName).equals("B")
o(onbeforeremove.callCount).equals(1) 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)
})
})
})
}) })

View file

@ -222,8 +222,8 @@ o.spec("onremove", function() {
text: textNode.nodeValue, text: textNode.nodeValue,
}) })
} }
actual = JSON.stringify(actual, null, ' ') actual = JSON.stringify(actual, null, " ")
expected = JSON.stringify(expected, null, ' ') expected = JSON.stringify(expected, null, " ")
return { return {
pass: actual === expected, pass: actual === expected,
message: message:
@ -232,21 +232,24 @@ o.spec("onremove", function() {
${actual}` ${actual}`
} }
}} }}
var finallyCB1
var finallyCB var finallyCB2
var C = createComponent({
view({children}){return children},
onbeforeremove(){
return {then(){}, finally: function (fcb) { finallyCB1 = fcb }}
}
})
function update(id, showParent, showChild) { function update(id, showParent, showChild) {
render(root, render(root,
m("div", m("div",
showParent && fragment( showParent && fragment(
"", // Required "", // Required
showChild && fragment( showChild && m(C, {
{ onbeforeremove: function () {
onbeforeremove: function () { return {then(){}, finally: function (fcb) { finallyCB2 = fcb }}
return {then(){}, finally: function (fcb) { finallyCB = fcb }}
},
}, },
m("div", id) }, m("div", id))
)
) )
) )
) )
@ -257,7 +260,8 @@ ${actual}`
["#text", ""], ["#text", ""],
["DIV", "1"], ["DIV", "1"],
])) ]))
o(finallyCB).equals(undefined) o(finallyCB1).equals(undefined)
o(finallyCB2).equals(undefined)
update("2", true, false) update("2", true, false)
@ -265,9 +269,10 @@ ${actual}`
["#text", ""], ["#text", ""],
["DIV", "1"], ["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) update("3", true, true)
@ -276,14 +281,16 @@ ${actual}`
["DIV", "1"], ["DIV", "1"],
["DIV", "3"], ["DIV", "3"],
])) ]))
o(finallyCB).equals(original) o(finallyCB1).equals(original1)
o(finallyCB2).equals(original2)
update("4", false, true) update("4", false, true)
o(root).satisfies(template([ o(root).satisfies(template([
["DIV", "1"], ["DIV", "1"],
])) ]))
o(finallyCB).equals(original) o(finallyCB1).equals(original1)
o(finallyCB2).equals(original2)
update("5", true, true) update("5", true, true)
@ -292,22 +299,26 @@ ${actual}`
["#text", ""], ["#text", ""],
["DIV", "5"], ["DIV", "5"],
])) ]))
o(finallyCB).equals(original) o(finallyCB1).equals(original1)
o(finallyCB2).equals(original2)
finallyCB() finallyCB1()
finallyCB2()
o(root).satisfies(template([ o(root).satisfies(template([
["#text", ""], ["#text", ""],
["DIV", "5"], ["DIV", "5"],
])) ]))
o(finallyCB).equals(original) o(finallyCB1).equals(original1)
o(finallyCB2).equals(original2)
update("6", true, true) update("6", true, true)
o(root).satisfies(template([ o(root).satisfies(template([
["#text", ""], ["#text", ""],
["DIV", "6"], ["DIV", "6"],
])) ]))
o(finallyCB).equals(original) o(finallyCB1).equals(original1)
o(finallyCB2).equals(original2)
}) })
}) })
}) })

View file

@ -99,7 +99,7 @@ module.exports = function(options) {
} }
} }
function removeChild(child) { 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'") throw new TypeError("Failed to execute removeChild, parameter is not of type 'Node'")
} }
var index = this.childNodes.indexOf(child) var index = this.childNodes.indexOf(child)