diff --git a/package-lock.json b/package-lock.json index 672b205b..780e6292 100644 --- a/package-lock.json +++ b/package-lock.json @@ -8,9 +8,6 @@ "name": "mithril", "version": "2.2.2", "license": "MIT", - "bin": { - "ospec": "ospec/bin/ospec" - }, "devDependencies": { "@alrra/travis-scripts": "^3.0.1", "@babel/parser": "^7.7.5", diff --git a/render/render.js b/render/render.js index 276cd89a..75a950b2 100644 --- a/render/render.js +++ b/render/render.js @@ -14,7 +14,6 @@ module.exports = function($window) { } var currentRedraw - var currentDOM var currentRender function getNameSpace(vnode) { @@ -323,9 +322,9 @@ module.exports = function($window) { if (start === end) break if (o.key !== ve.key || oe.key !== v.key) break topSibling = getNextSibling(old, oldStart, nextSibling) - moveNode(parent, oe, topSibling) + moveDOM(parent, oe, topSibling) if (oe !== v) updateNode(parent, oe, v, hooks, topSibling, ns) - if (++start <= --end) moveNode(parent, o, nextSibling) + if (++start <= --end) moveDOM(parent, o, nextSibling) if (o !== ve) updateNode(parent, o, ve, hooks, nextSibling, ns) if (ve.dom != null) nextSibling = ve.dom oldStart++; oldEnd-- @@ -377,7 +376,7 @@ module.exports = function($window) { if (oldIndices[i-start] === -1) createNode(parent, v, hooks, ns, nextSibling) else { if (lisIndices[li] === i - start) li-- - else moveNode(parent, v, nextSibling) + else moveDOM(parent, v, nextSibling) } if (v.dom != null) nextSibling = vnodes[i].dom } @@ -547,7 +546,7 @@ module.exports = function($window) { } // This handles fragments with zombie children (removed from vdom, but persisted in DOM through onbeforeremove) - function moveNode(parent, vnode, nextSibling) { + function moveDOM(parent, vnode, nextSibling) { if (vnode.dom != null) { var target if (vnode.domSize == null) { @@ -611,25 +610,37 @@ module.exports = function($window) { // If we can, try to fast-path it and avoid all the overhead of awaiting if (!mask) { onremove(vnode) - removeDOM(parent, vnode, undefined) + removeDOM(parent, vnode, generation) } else { generation = currentRender 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) { - checkState(vnode, original) - onremove(vnode) - removeDOM(parent, vnode, generation) - } } - } - } if (stateResult != null) { - stateResult.finally(finalizer(1, 2)) + stateResult.finally(function () { + // eslint-disable-next-line no-bitwise + if (mask & 1) { + // eslint-disable-next-line no-bitwise + mask &= 2 + if (!mask) { + checkState(vnode, original) + onremove(vnode) + removeDOM(parent, vnode, generation) + } + } + }) } if (attrsResult != null) { - attrsResult.finally(finalizer(2, 1)) + attrsResult.finally(function () { + // eslint-disable-next-line no-bitwise + if (mask & 2) { + // eslint-disable-next-line no-bitwise + mask &= 1 + if (!mask) { + checkState(vnode, original) + onremove(vnode) + removeDOM(parent, vnode, generation) + } + } + }) } } } @@ -902,6 +913,8 @@ module.exports = function($window) { return true } + var currentDOM + return function(dom, vnodes, redraw) { if (!dom) throw new TypeError("DOM element being rendered to does not exist.") if (currentDOM != null && dom.contains(currentDOM)) { diff --git a/render/tests/test-onbeforeremove.js b/render/tests/test-onbeforeremove.js index 96a27b74..b5621e42 100644 --- a/render/tests/test-onbeforeremove.js +++ b/render/tests/test-onbeforeremove.js @@ -74,17 +74,18 @@ o.spec("onbeforeremove", function() { }) } }) - o("calls remove after onbeforeremove resolves", function(done) { + o("calls onremove after onbeforeremove resolves", function(done) { var spy = o.spy() - var vnode = fragment({onbeforeremove: remove, onremove: spy}, "a") + var vnode = fragment({onbeforeremove: onbeforeremove, onremove: spy}, "a") render(root, vnode) render(root, []) - function remove(node) { + function onbeforeremove(node) { o(node).equals(vnode) o(root.childNodes.length).equals(1) o(root.firstChild).equals(vnode.dom) + o(spy.callCount).equals(0) callAsync(function() { o(root.childNodes.length).equals(0) @@ -103,17 +104,6 @@ o.spec("onbeforeremove", function() { o(vnode.dom.onbeforeremove).equals(undefined) o(vnode.dom.attributes["onbeforeremove"]).equals(undefined) }) - o("does not recycle when there's an onbeforeremove", function() { - var remove = function() {} - var vnode = m("div", {key: 1, onbeforeremove: remove}) - var updated = m("div", {key: 1, onbeforeremove: remove}) - - render(root, vnode) - render(root, []) - render(root, updated) - - o(vnode.dom).notEquals(updated.dom) - }) o("does not leave elements out of order during removal", function(done) { var remove = function() {return Promise.resolve()} var vnodes = [m("div", {key: 1, onbeforeremove: remove}, "1"), m("div", {key: 2, onbeforeremove: remove}, "2")] diff --git a/render/tests/test-onremove.js b/render/tests/test-onremove.js index 2a1d36ac..bf4800ff 100644 --- a/render/tests/test-onremove.js +++ b/render/tests/test-onremove.js @@ -194,9 +194,10 @@ o.spec("onremove", function() { o(onremove.callCount).equals(1) }) // Warning: this test is complicated because it's replicating a race condition. - o("removes correct nodes when child delays removal, parent removes, then child resolves", function () { + o("removes correct nodes in fragment when child delays removal, parent removes, then child resolves", function () { // Custom assertion - we need to test the entire tree for consistency. - function template(tpl) {return function (root) { + + const template = (tpl) => (root) => { var expected = [] for (var i = 0; i < tpl.length; i++) { @@ -231,7 +232,7 @@ o.spec("onremove", function() { expected, got ${actual}` } - }} + } var finallyCB1 var finallyCB2 var C = createComponent({ @@ -241,84 +242,137 @@ ${actual}` } }) function update(id, showParent, showChild) { + const removeParent = o.spy() + const removeSyncChild = o.spy() + const removeAsyncChild = o.spy() + render(root, m("div", showParent && fragment( - "", // Required + {onremove: removeParent}, + m("a", {onremove: removeSyncChild}, "sync child"), showChild && m(C, { onbeforeremove: function () { return {then(){}, finally: function (fcb) { finallyCB2 = fcb }} }, + onremove: removeAsyncChild }, m("div", id)) ) ) ) + return {removeAsyncChild,removeParent, removeSyncChild} } - update("1", true, true) + const hooks1 = update("1", true, true) o(root).satisfies(template([ - ["#text", ""], + ["A", "sync child"], ["DIV", "1"], ])) o(finallyCB1).equals(undefined) o(finallyCB2).equals(undefined) - update("2", true, false) + const hooks2 = update("2", true, false) o(root).satisfies(template([ - ["#text", ""], + ["A", "sync child"], ["DIV", "1"], ])) + o(typeof finallyCB1).equals("function") + o(typeof finallyCB2).equals("function") var original1 = finallyCB1 var original2 = finallyCB2 - update("3", true, true) + const hooks3 = update("3", true, true) o(root).satisfies(template([ - ["#text", ""], + ["A", "sync child"], ["DIV", "1"], ["DIV", "3"], ])) + + o(hooks3.removeParent.callCount).equals(0) + o(hooks3.removeSyncChild.callCount).equals(0) + o(hooks3.removeAsyncChild.callCount).equals(0) o(finallyCB1).equals(original1) o(finallyCB2).equals(original2) - update("4", false, true) + const hooks4 = update("4", false, true) o(root).satisfies(template([ ["DIV", "1"], ])) + + o(hooks3.removeParent.callCount).equals(1) + o(hooks3.removeSyncChild.callCount).equals(1) + o(hooks3.removeAsyncChild.callCount).equals(1) + o(hooks3.removeParent.args[0].tag).equals("[") o(finallyCB1).equals(original1) o(finallyCB2).equals(original2) - update("5", true, true) + const hooks5 = update("5", true, true) + o(root).satisfies(template([ ["DIV", "1"], - ["#text", ""], + ["A", "sync child"], ["DIV", "5"], ])) o(finallyCB1).equals(original1) o(finallyCB2).equals(original2) + o(hooks1.removeAsyncChild.callCount).equals(0) + finallyCB1() + + o(hooks1.removeAsyncChild.callCount).equals(0) + finallyCB2() + o(hooks1.removeAsyncChild.callCount).equals(1) + o(root).satisfies(template([ - ["#text", ""], + ["A", "sync child"], ["DIV", "5"], ])) o(finallyCB1).equals(original1) o(finallyCB2).equals(original2) - update("6", true, true) + const hooks6 = update("6", true, true) + o(root).satisfies(template([ - ["#text", ""], + ["A", "sync child"], ["DIV", "6"], ])) o(finallyCB1).equals(original1) o(finallyCB2).equals(original2) + + // final tally + o(hooks1.removeParent.callCount).equals(0) + o(hooks1.removeSyncChild.callCount).equals(0) + o(hooks1.removeAsyncChild.callCount).equals(1) + + o(hooks2.removeParent.callCount).equals(0) + o(hooks2.removeSyncChild.callCount).equals(0) + o(hooks2.removeAsyncChild.callCount).equals(0) + + o(hooks3.removeParent.callCount).equals(1) + o(hooks3.removeSyncChild.callCount).equals(1) + o(hooks3.removeAsyncChild.callCount).equals(1) + + o(hooks4.removeParent.callCount).equals(0) + o(hooks4.removeSyncChild.callCount).equals(0) + o(hooks4.removeAsyncChild.callCount).equals(0) + + o(hooks5.removeParent.callCount).equals(0) + o(hooks5.removeSyncChild.callCount).equals(0) + o(hooks5.removeAsyncChild.callCount).equals(0) + + o(hooks6.removeParent.callCount).equals(0) + o(hooks6.removeSyncChild.callCount).equals(0) + o(hooks6.removeAsyncChild.callCount).equals(0) + }) }) })