From 9e9b89d90003224a7d6759edd6536f4c69fe4924 Mon Sep 17 00:00:00 2001 From: Isiah Meadows Date: Wed, 3 Jul 2019 04:53:45 -0400 Subject: [PATCH] Fix #2067 (#2447) * Fix #2067 * Add PR number [skip ci] --- docs/change-log.md | 2 + render/render.js | 10 +++ render/tests/test-onbeforeupdate.js | 96 +++++++++++++++++++++++++++++ 3 files changed, 108 insertions(+) diff --git a/docs/change-log.md b/docs/change-log.md index d75c84af..061c8ed5 100644 --- a/docs/change-log.md +++ b/docs/change-log.md @@ -94,6 +94,8 @@ - The raw bundle itself remains accessible at `mithril.js`, and is *not* browser-wrapped. - Note: this *will* increase overhead with bundlers like Webpack, Rollup, and Browserify. - request: autoredraw support fixed for `async`/`await` in Chrome ([#2428](https://github.com/MithrilJS/mithril.js/pull/2428) [@isiahmeadows](https://github.com/isiahmeadows)) +- render: fix when attrs change with `onbeforeupdate` returning false, then remaining the same on next redraw ([#2447](https://github.com/MithrilJS/mithril.js/pull/2447) [@isiahmeadows](https://github.com/isiahmeadows)) +- render: fix internal error when `onbeforeupdate` returns false and then true with new child tree ([#2447](https://github.com/MithrilJS/mithril.js/pull/2447) [@isiahmeadows](https://github.com/isiahmeadows)) --- diff --git a/render/render.js b/render/render.js index f364d731..43d0ab31 100644 --- a/render/render.js +++ b/render/render.js @@ -885,6 +885,16 @@ module.exports = function($window) { vnode.dom = old.dom vnode.domSize = old.domSize vnode.instance = old.instance + // One would think having the actual latest attributes would be ideal, + // but it doesn't let us properly diff based on our current internal + // representation. We have to save not only the old DOM info, but also + // the attributes used to create it, as we diff *that*, not against the + // DOM directly (with a few exceptions in `setAttr`). And, of course, we + // need to save the children and text as they are conceptually not + // unlike special "attributes" internally. + vnode.attrs = old.attrs + vnode.children = old.children + vnode.text = old.text return true } diff --git a/render/tests/test-onbeforeupdate.js b/render/tests/test-onbeforeupdate.js index 9ff828b6..6c5adac8 100644 --- a/render/tests/test-onbeforeupdate.js +++ b/render/tests/test-onbeforeupdate.js @@ -306,4 +306,100 @@ o.spec("onbeforeupdate", function() { }) }) }) + + // https://github.com/MithrilJS/mithril.js/issues/2067 + o.spec("after prevented update", function() { + o("old attributes are retained", function() { + render(root, [ + {tag: "div", attrs: {"id": "foo", onbeforeupdate: function() { return true }}, children: []}, + ]) + render(root, [ + {tag: "div", attrs: {"id": "bar", onbeforeupdate: function() { return false }}, children: []}, + ]) + render(root, [ + {tag: "div", attrs: {"id": "bar", onbeforeupdate: function() { return true }}, children: []}, + ]) + o(root.firstChild.attributes["id"].value).equals("bar") + }) + o("old children is retained", function() { + render(root, [ + {tag: "div", attrs: {onbeforeupdate: function() { return true }}, children: [ + {tag: "div", attrs: {}, children: []} + ]}, + ]) + render(root, [ + {tag: "div", attrs: {onbeforeupdate: function() { return false }}, children: [ + {tag: "div", attrs: {}, children: [{tag: "div", attrs: {}, children: []}]} + ]}, + ]) + render(root, [ + {tag: "div", attrs: {onbeforeupdate: function() { return true }}, children: [ + {tag: "div", attrs: {}, children: [{tag: "div", attrs: {}, children: []}]} + ]}, + ]) + o(root.firstChild.firstChild.childNodes.length).equals(1) + }) + o("old text is retained", function() { + render(root, [ + {tag: "div", attrs: {onbeforeupdate: function() { return true }}, children: [ + {tag: "div", attrs: {}, text: ""} + ]}, + ]) + render(root, [ + {tag: "div", attrs: {onbeforeupdate: function() { return false }}, children: [ + {tag: "div", attrs: {}, text: "foo"} + ]}, + ]) + render(root, [ + {tag: "div", attrs: {onbeforeupdate: function() { return true }}, children: [ + {tag: "div", attrs: {}, text: "foo"} + ]}, + ]) + o(root.firstChild.firstChild.firstChild.nodeValue).equals("foo") + }) + o("updating component children doesn't error", function() { + var Child = { + view(v) { + return {tag: "div", attrs: {}, children: [ + v.attrs.foo ? {tag: "div", attrs: {}, children: []} : null + ]} + } + } + + render(root, [ + {tag: "div", attrs: {onbeforeupdate: function() { return true }}, children: [ + {tag: Child, attrs: {foo: false}, children: []}, + ]}, + ]) + render(root, [ + {tag: "div", attrs: {onbeforeupdate: function() { return false }}, children: [ + {tag: Child, attrs: {foo: false}, children: []}, + ]}, + ]) + render(root, [ + {tag: "div", attrs: {onbeforeupdate: function() { return true }}, children: [ + {tag: Child, attrs: {foo: true}, children: []}, + ]}, + ]) + o(root.firstChild.firstChild.childNodes.length).equals(1) + }) + o("adding dom children doesn't error", function() { + render(root, [ + {tag: "div", attrs: {onbeforeupdate: function() { return true }}, children: [ + {tag: "div", attrs: {}, children: []} + ]}, + ]) + render(root, [ + {tag: "div", attrs: {onbeforeupdate: function() { return false }}, children: [ + {tag: "div", attrs: {}, children: []} + ]}, + ]) + render(root, [ + {tag: "div", attrs: {onbeforeupdate: function() { return true }}, children: [ + {tag: "div", attrs: {}, children: [{tag: "div", attrs: {}, children: []}]} + ]}, + ]) + o(root.firstChild.firstChild.childNodes.length).equals(1) + }) + }) })