Fix #2067 (#2447)

* Fix #2067

* Add PR number [skip ci]

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))
 
 ---
 

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
 	}
 

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)
+		})
+	})
 })