fix attribute syncing for input, select, textarea

optimize events

README.md

@@ -6,13 +6,13 @@ This rewrite aims to fix longstanding API design issues, significantly improve p
 
 ## Status
 
-Code still is in flux. Most notably, thunks (`{subtree: "retain"}`) are currently not implemented yet and there are several use cases that still need to be polished. DO NOT USE IN PRODUCTION YET!
+Code still is in flux. Most notably, there's no promise polyfill yet and there are several use cases that still need to be polished. DO NOT USE IN PRODUCTION YET!
 
 Some examples of usage can be found in the [examples](examples) folder. [ThreadItJS](http://cdn.rawgit.com/lhorie/mithril.js/rewrite/examples/threaditjs/index.html) has the largest API surface coverage and comments indicating pending issues in framework usability. Note that the APIs those examples use may not become the final public API points in v1.0.
 
 ## Performance
 
-Mithril's virtual DOM engine is now around 400 lines of well organized code and it implements a modern search space reduction diff algorithm and a DOM recycling mechanism, which translate to top-of-class performance. See the [dbmon implementation (non-optimized)](http://cdn.rawgit.com/lhorie/mithril.js/rewrite/examples/dbmonster/mithril/index.html) (for comparison, here are optimized dbmon implementations for [React v15.0.2](http://cdn.rawgit.com/lhorie/mithril.js/rewrite/examples/dbmonster/react/index.html) and [Angular v2.0.0-beta.17](http://cdn.rawgit.com/lhorie/mithril.js/rewrite/examples/dbmonster/angular/index.html)).
+Mithril's virtual DOM engine is less than 500 lines of well organized code and it implements a modern search space reduction diff algorithm and a DOM recycling mechanism, which translate to top-of-class performance. See the [dbmon implementation (non-optimized)](http://cdn.rawgit.com/lhorie/mithril.js/rewrite/examples/dbmonster/mithril/index.html) (for comparison, here are optimized dbmon implementations for [React v15.0.2](http://cdn.rawgit.com/lhorie/mithril.js/rewrite/examples/dbmonster/react/index.html) and [Angular v2.0.0-beta.17](http://cdn.rawgit.com/lhorie/mithril.js/rewrite/examples/dbmonster/angular/index.html)).
 
 ## Lifecycle methods and Animation Support
 

render/render.js

@@ -82,9 +82,7 @@ module.exports = function($window, onevent) {
 		if (vnode.children != null) {
 			var children = vnode.children
 			createNodes(element, children, 0, children.length, hooks, null)
-			if (tag === "select" && "value" in attrs) {
-				setAttrs(vnode, { value: attrs.value })
-			}
+			setLateAttrs(vnode)
 		}
 		return element
 	}
@@ -214,6 +212,10 @@ module.exports = function($window, onevent) {
 	}
 	function updateElement(old, vnode, hooks) {
 		var element = vnode.dom = old.dom
+		if (vnode.tag === "textarea") {
+			if (vnode.attrs == null) vnode.attrs = {}
+			if (vnode.text != null) vnode.attrs.value = vnode.text //FIXME handle multiple children
+		}
 		updateAttrs(vnode, old.attrs, vnode.attrs)
 		if (old.text != null && vnode.text != null && vnode.text !== "") {
 			if (old.text.toString() !== vnode.text.toString()) old.dom.firstChild.nodeValue = vnode.text
@@ -343,24 +345,12 @@ module.exports = function($window, onevent) {
 	function setAttr(vnode, key, old, value) {
 		//TODO test input undo history
 		var element = vnode.dom
-		if (key === "key" || old === value || typeof value === "undefined" || isLifecycleMethod(key)) return
+		if (key === "key" || (!isFormAttribute(vnode, key) && old === value) || typeof value === "undefined" || isLifecycleMethod(key)) return
 		var nsLastIndex = key.indexOf(":")
 		if (nsLastIndex > -1 && key.substr(0, nsLastIndex) === "xlink") {
 			element.setAttributeNS("http://www.w3.org/1999/xlink", key.slice(nsLastIndex + 1), value)
 		}
-		else if (key[0] === "o" && key[1] === "n" && typeof value === "function") {
-			var eventName = key.slice(2)
-			if (vnode.events === undefined) vnode.events = {}
-			if (vnode.events[key] != null) {
-				element.removeEventListener(eventName, vnode.events[key], false)
-			}
-			vnode.events[key] = function(e) {
-				var result = value.call(element, e)
-				if (typeof onevent === "function") onevent.call(element, e)
-				return result
-			}
-			element.addEventListener(eventName, vnode.events[key], false)
-		}
+		else if (key[0] === "o" && key[1] === "n" && typeof value === "function") updateEvent(vnode, key, value)
 		else if (key === "style") updateStyle(element, old, value)
 		else if (key in element && !isAttribute(key) && vnode.ns === undefined) element[key] = value
 		else {
@@ -371,10 +361,17 @@ module.exports = function($window, onevent) {
 			else element.setAttribute(key, value)
 		}
 	}
+	function setLateAttrs(vnode) {
+		var attrs = vnode.attrs
+		if (vnode.tag === "select" && attrs != null) {
+			if ("value" in attrs) setAttr(vnode, "value", null, attrs.value)
+			if ("selectedIndex" in attrs) setAttr(vnode, "selectedIndex", null, attrs.selectedIndex)
+		}
+	}
 	function updateAttrs(vnode, old, attrs) {
 		if (attrs != null) {
 			for (var key in attrs) {
-				setAttr(vnode, key, old[key], attrs[key])
+				setAttr(vnode, key, old && old[key], attrs[key])
 			}
 		}
 		if (old != null) {
@@ -385,6 +382,9 @@ module.exports = function($window, onevent) {
 			}
 		}
 	}
+	function isFormAttribute(vnode, attr) {
+		return attr === "value" || attr === "checked" || attr === "selectedIndex" || attr === "selected" && vnode.dom === $doc.activeElement
+	}
 	function isLifecycleMethod(attr) {
 		return attr === "oninit" || attr === "oncreate" || attr === "onupdate" || attr === "onremove" || attr === "onbeforeremove" || attr === "shouldUpdate"
 	}
@@ -408,6 +408,24 @@ module.exports = function($window, onevent) {
 			}
 		}
 	}
+	
+	//event
+	function updateEvent(vnode, key, value) {
+		var element = vnode.dom
+		var callback = function(e) {
+			var result = value.call(element, e)
+			if (typeof onevent === "function") onevent.call(element, e)
+			return result
+		}
+		if (key in element) element[key] = callback
+		else {
+			var eventName = key.slice(2)
+			if (vnode.events === undefined) vnode.events = {}
+			if (vnode.events[key] != null) element.removeEventListener(eventName, vnode.events[key], false)
+			vnode.events[key] = callback
+			element.addEventListener(eventName, vnode.events[key], false)
+		}
+	}
 
 	//lifecycle
 	function initLifecycle(source, vnode, hooks) {

render/tests/test-input.js

@@ -4,23 +4,138 @@ var o = require("../../ospec/ospec")
 var domMock = require("../../test-utils/domMock")
 var vdom = require("../../render/render")
 
-o.spec("input", function() {
+o.spec("form inputs", function() {
 	var $window, root, render
 	o.beforeEach(function() {
 		$window = domMock()
-		root = $window.document.body
 		render = vdom($window).render
+		root = $window.document.createElement("div")
+		$window.document.body.appendChild(root)
+	})
+	o.afterEach(function() {
+		while (root.firstChild) root.removeChild(root.firstChild)
 	})
 
-	o("maintains focus after move", function() {
-		var input = {tag: "input", key: 1}
-		var a = {tag: "a", key: 2}
-		var b = {tag: "b", key: 3}
+	o.spec("input", function() {
+		o("maintains focus after move", function() {
+			var input = {tag: "input", key: 1}
+			var a = {tag: "a", key: 2}
+			var b = {tag: "b", key: 3}
+			
+			render(root, [input, a, b])
+			input.dom.focus()
+			render(root, [a, input, b])
+			
+			o($window.document.activeElement).equals(input.dom)
+		})
 		
-		render(root, [input, a, b])
-		input.dom.focus()
-		render(root, [a, input, b])
+		o("syncs input value if DOM value differs from vdom value", function() {
+			var input = {tag: "input", attrs: {value: "aaa", oninput: function() {}}}
+			var updated = {tag: "input", attrs: {value: "aaa", oninput: function() {}}}
+			
+			render(root, [input])
+			
+			//simulate user typing
+			var e = $window.document.createEvent("KeyboardEvent")
+			e.initEvent("input", true, true)
+			input.dom.focus()
+			input.dom.value += "a"
+			input.dom.dispatchEvent(e)
+			
+			//re-render may use same vdom value as previous render call
+			render(root, [updated])
+			
+			o(updated.dom.value).equals("aaa")
+		})
 		
-		o($window.document.activeElement).equals(input.dom)
+		o("syncs input checked attribute if DOM value differs from vdom value", function() {
+			var input = {tag: "input", attrs: {type: "checkbox", checked: true, onclick: function() {}}}
+			var updated = {tag: "input", attrs: {type: "checkbox", checked: true, onclick: function() {}}}
+			
+			render(root, [input])
+			
+			//simulate user clicking checkbox
+			var e = $window.document.createEvent("MouseEvents")
+			e.initEvent("click", true, true)
+			input.dom.focus()
+			input.dom.dispatchEvent(e)
+			
+			//re-render may use same vdom value as previous render call
+			render(root, [updated])
+			
+			o(updated.dom.checked).equals(true)
+		})
+	})
+	
+	o.spec("select", function() {
+		o("select works without attributes", function() {
+			var select = {tag: "select", children: [
+				{tag: "option", attrs: {value: "a"}, text: "aaa"},
+			]}
+			
+			render(root, [select])
+			
+			o(select.dom.value).equals("a")
+			o(select.dom.selectedIndex).equals(0)
+		})
+		
+		o("select yields invalid value without children", function() {
+			var select = {tag: "select", attrs: {value: "a"}}
+			
+			render(root, [select])
+			
+			o(select.dom.value).equals("")
+			o(select.dom.selectedIndex).equals(-1)
+		})
+		
+		o("select value is set correctly on first render", function() {
+			var select = {tag: "select", attrs: {value: "b"}, children: [
+				{tag: "option", attrs: {value: "a"}, text: "aaa"},
+				{tag: "option", attrs: {value: "b"}, text: "bbb"},
+				{tag: "option", attrs: {value: "c"}, text: "ccc"},
+			]}
+			
+			render(root, [select])
+			
+			o(select.dom.value).equals("b")
+			o(select.dom.selectedIndex).equals(1)
+		})
+		
+		o("syncs select value if DOM value differs from vdom value", function() {
+			function makeSelect() {
+				return {tag: "select", attrs: {value: "b"}, children: [
+					{tag: "option", attrs: {value: "a"}, text: "aaa"},
+					{tag: "option", attrs: {value: "b"}, text: "bbb"},
+					{tag: "option", attrs: {value: "c"}, text: "ccc"},
+				]}
+			}
+			
+			render(root, [makeSelect()])
+			
+			//simulate user selecting option
+			root.firstChild.value = "c"
+			root.firstChild.focus()
+			
+			//re-render may use same vdom value as previous render call
+			render(root, [makeSelect()])
+			
+			o(root.firstChild.value).equals("b")
+			o(root.firstChild.selectedIndex).equals(1)
+		})
+	})
+	
+	o.spec("textarea", function() {
+		o("updates after user input", function() {
+			render(root, [{tag: "textarea", text: "aaa"}])
+			
+			//simulate typing
+			root.firstChild.value = "bbb"
+			
+			//re-render may occur after value attribute is touched
+			render(root, [{tag: "textarea", text: "ccc"}])
+			
+			o(root.firstChild.value).equals("ccc")
+			//FIXME should fail if fix is commented out
+		})
 	})
 })

render/tests/test-updateElement.js

@@ -34,6 +34,17 @@ o.spec("updateElement", function() {
 		o(updated.dom).equals(root.firstChild)
 		o(updated.dom.attributes["title"].nodeValue).equals("d")
 	})
+	o("adds attr from empty attrs", function() {
+		var vnode = {tag: "a"}
+		var updated = {tag: "a", attrs: {title: "d"}}
+		
+		render(root, [vnode])
+		render(root, [updated])
+		
+		o(updated.dom).equals(vnode.dom)
+		o(updated.dom).equals(root.firstChild)
+		o(updated.dom.attributes["title"].nodeValue).equals("d")
+	})
 	o("removes attr", function() {
 		var vnode = {tag: "a", attrs: {id: "b", title: "d"}}
 		var updated = {tag: "a", attrs: {id: "c"}}

test-input.html

@@ -0,0 +1,40 @@
+<!doctype html>
+<html>
+	<head></head>
+	<body>
+		<textarea id="t">aaa</textarea>
+		<select multiple id="aaa">
+			<option value="a">aaa</option>
+			<option value="b">bbb</option>
+			<option value="c">ccc</option>
+		</select>
+		<div id="root"></div>
+		<pre id="a"></pre>
+		<script src="./module/module.js"></script>
+		<script src="./render/node.js"></script>
+		<script src="./render/hyperscript.js"></script>
+		<script src="./render/render.js"></script>
+		<script type="text/javascript">
+var m = require("./render/hyperscript")
+var render = require("./render/render")(window, run).render
+
+var value = "asd"
+
+function run() {
+	console.log("rendering...")
+	render(root, [
+		m("textarea", {oninput: (e) => e}, value)
+	])
+}
+
+run()
+
+//setInterval(()=> console.log(document.activeElement), 1000)
+
+var el = document.createElement("br")
+var txt = document.createTextNode("ccc")
+t.appendChild(el)
+t.appendChild(txt)
+		</script>
+	</body>
+</html>

test-utils/domMock.js

@@ -163,6 +163,10 @@ module.exports = function() {
 						}
 					},
 					dispatchEvent: function(e) {
+						if (this.nodeName === "INPUT" && this.attributes["type"] != null && this.attributes["type"].nodeValue === "checkbox" && e.type === "click") {
+							this.checked = !this.checked
+						}
+						
 						e.target = this
 						if (events[e.type] != null) {
 							for (var i = 0; i < events[e.type].length; i++) {
@@ -172,6 +176,7 @@ module.exports = function() {
 						if (typeof this["on" + e.type] === "function" && !isModernEvent(e.type)) this["on" + e.type](e)
 					},
 				}
+				
 				if (element.nodeName === "A") {
 					var href
 					Object.defineProperty(element, "href", {
@@ -180,6 +185,7 @@ module.exports = function() {
 						enumerable: true,
 					})
 				}
+				
 				if (element.nodeName === "INPUT") {
 					var checked
 					Object.defineProperty(element, "checked", {
@@ -187,6 +193,98 @@ module.exports = function() {
 						set: function(value) {checked = Boolean(value)},
 						enumerable: true,
 					})
+					
+					element.value = ""
+				}
+				
+				if (element.nodeName === "TEXTAREA") {
+					var value
+					Object.defineProperty(element, "value", {
+						get: function() {
+							return value != null ? value :
+								this.firstChild ? this.firstChild.nodeValue : ""
+						},
+						set: function(v) {value = v},
+						enumerable: true,
+					})
+				}
+				
+				function getOptions(element) {
+					var options = []
+					for (var i = 0; i < element.childNodes.length; i++) {
+						if (element.childNodes[i].nodeName === "OPTION") options.push(element.childNodes[i])
+						else if (element.childNodes[i].nodeName === "OPTGROUP") options = options.concat(getOptions(element.childNodes[i]))
+					}
+					return options
+				}
+				function getOptionValue(element) {
+					return element.attributes["value"] != null ?
+						element.attributes["value"].nodeValue :
+						element.firstChild != null ? element.firstChild.nodeValue : ""
+				}
+				if (element.nodeName === "SELECT") {
+					var selectedValue, selectedIndex = 0
+					Object.defineProperty(element, "selectedIndex", {
+						get: function() {return getOptions(this).length > 0 ? selectedIndex : -1},
+						set: function(value) {
+							var options = getOptions(this)
+							if (value >= 0 && value < options.length) {
+								selectedValue = getOptionValue(options[selectedIndex])
+								selectedIndex = value
+							}
+							else {
+								selectedValue = ""
+								selectedIndex = -1
+							}
+						},
+						enumerable: true,
+					})
+					Object.defineProperty(element, "value", {
+						get: function() {
+							if (this.selectedIndex > -1) return getOptionValue(getOptions(this)[this.selectedIndex])
+							return ""
+						},
+						set: function(value) {
+							var options = getOptions(this)
+							var stringValue = String(value)
+							for (var i = 0; i < options.length; i++) {
+								if (getOptionValue(options[i]) === stringValue) {
+									selectedValue = stringValue
+									selectedIndex = i
+									return
+								}
+							}
+							selectedValue = stringValue
+							selectedIndex = -1
+						},
+						enumerable: true,
+					})
+				}
+				if (element.nodeName === "OPTION") {
+					Object.defineProperty(element, "value", {
+						get: function() {return getOptionValue(this)},
+						set: function(value) {
+							this.setAttribute("value", value)
+						},
+						enumerable: true,
+					})
+					Object.defineProperty(element, "selected", {
+						get: function() {
+							var options = getOptions(this.parentNode)
+							var index = options.indexOf(this)
+							if (index > -1) return index === this.parentNode.selectedIndex
+							return false
+						},
+						set: function(value) {
+							if (value) {
+								var options = getOptions(this.parentNode)
+								var index = options.indexOf(this)
+								if (index > -1) this.parentNode.selectedIndex = index
+							}
+							else this.parentNode.selectedIndex = 0
+						},
+						enumerable: true,
+					})
 				}
 				return element
 			},

test-utils/tests/test-domMock.js

@@ -553,7 +553,7 @@ o.spec("domMock", function() {
 		})
 	})
 	o.spec("attributes", function() {
-		o.spec("link href", function() {
+		o.spec("a[href]", function() {
 			o("is empty string if no attribute", function() {
 				var a = $document.createElement("a")
 				
@@ -575,7 +575,7 @@ o.spec("domMock", function() {
 				o(a.attributes["href"].nodeValue).equals("")
 			})
 		})
-		o.spec("input checked", function() {
+		o.spec("input[checked]", function() {
 			o("only exists in input elements", function() {
 				var input = $document.createElement("input")
 				var a = $document.createElement("a")
@@ -616,6 +616,195 @@ o.spec("domMock", function() {
 				
 				o(input.checked).equals(true)
 			})
+			o("toggles on click", function() {
+				var input = $document.createElement("input")
+				input.setAttribute("type", "checkbox")
+				input.checked = false
+				
+				var e = $document.createEvent("MouseEvents")
+				e.initEvent("click", true, true)
+				input.dispatchEvent(e)
+				
+				o(input.checked).equals(true)
+			})
+		})
+		o.spec("input[value]", function() {
+			o("only exists in input elements", function() {
+				var input = $document.createElement("input")
+				var a = $document.createElement("a")
+				
+				o("value" in input).equals(true)
+				o("value" in a).equals(false)
+			})
+		})
+		o.spec("textarea[value]", function() {
+			o("reads from child if no value", function() {
+				var input = $document.createElement("textarea")
+				input.appendChild($document.createTextNode("aaa"))
+				
+				o(input.value).equals("aaa")
+			})
+			o("ignores child if value set", function() {
+				var input = $document.createElement("textarea")
+				input.value = "aaa"
+				input.setAttribute("value", "bbb")
+				
+				o(input.value).equals("aaa")
+			})
+		})
+		o.spec("select[value] and select[selectedIndex]", function() {
+			o("only exist in select elements", function() {
+				var select = $document.createElement("select")
+				var a = $document.createElement("a")
+				
+				o("value" in select).equals(true)
+				o("value" in a).equals(false)
+				
+				o("selectedIndex" in select).equals(true)
+				o("selectedIndex" in a).equals(false)
+			})
+			o("value defaults to value at first index", function() {
+				var select = $document.createElement("select")
+				
+				var option1 = $document.createElement("option")
+				option1.setAttribute("value", "a")
+				select.appendChild(option1)
+				
+				var option2 = $document.createElement("option")
+				option2.setAttribute("value", "b")
+				select.appendChild(option2)
+				
+				o(select.value).equals("a")
+				o(select.selectedIndex).equals(0)
+			})
+			o("value falls back to child nodeValue if no attribute", function() {
+				var select = $document.createElement("select")
+				
+				var option1 = $document.createElement("option")
+				option1.appendChild($document.createTextNode("a"))
+				select.appendChild(option1)
+				
+				o(select.value).equals("a")
+				o(select.selectedIndex).equals(0)
+			})
+			o("value defaults to invalid if no options", function() {
+				var select = $document.createElement("select")
+				
+				o(select.value).equals("")
+				o(select.selectedIndex).equals(-1)
+			})
+			o("setting valid value works", function() {
+				var select = $document.createElement("select")
+				
+				var option1 = $document.createElement("option")
+				option1.setAttribute("value", "a")
+				select.appendChild(option1)
+				
+				var option2 = $document.createElement("option")
+				option2.setAttribute("value", "b")
+				select.appendChild(option2)
+				
+				select.value = "b"
+				
+				o(select.value).equals("b")
+				o(select.selectedIndex).equals(1)
+			})
+			o("setting valid selectedIndex works", function() {
+				var select = $document.createElement("select")
+				
+				var option1 = $document.createElement("option")
+				option1.setAttribute("value", "a")
+				select.appendChild(option1)
+				
+				var option2 = $document.createElement("option")
+				option2.setAttribute("value", "b")
+				select.appendChild(option2)
+				
+				select.selectedIndex = 1
+				
+				o(select.value).equals("b")
+				o(select.selectedIndex).equals(1)
+			})
+			o("setting option[selected] works", function() {
+				var select = $document.createElement("select")
+				
+				var option1 = $document.createElement("option")
+				option1.setAttribute("value", "a")
+				select.appendChild(option1)
+				
+				var option2 = $document.createElement("option")
+				option2.setAttribute("value", "b")
+				select.appendChild(option2)
+				
+				select.childNodes[1].selected = true
+				
+				o(select.value).equals("b")
+				o(select.selectedIndex).equals(1)
+			})
+			o("setting invalid value yields a selectedIndex of -1 and value of empty string", function() {
+				var select = $document.createElement("select")
+				
+				var option1 = $document.createElement("option")
+				option1.setAttribute("value", "a")
+				select.appendChild(option1)
+				
+				var option2 = $document.createElement("option")
+				option2.setAttribute("value", "b")
+				select.appendChild(option2)
+				
+				select.value = "c"
+				
+				o(select.value).equals("")
+				o(select.selectedIndex).equals(-1)
+			})
+			o("setting invalid selectedIndex yields a selectedIndex of -1 and value of empty string", function() {
+				var select = $document.createElement("select")
+				
+				var option1 = $document.createElement("option")
+				option1.setAttribute("value", "a")
+				select.appendChild(option1)
+				
+				var option2 = $document.createElement("option")
+				option2.setAttribute("value", "b")
+				select.appendChild(option2)
+				
+				select.selectedIndex = -2
+				
+				o(select.value).equals("")
+				o(select.selectedIndex).equals(-1)
+			})
+			o("setting invalid value yields a selectedIndex of -1 and value of empty string even when there's an option whose value is empty string", function() {
+				var select = $document.createElement("select")
+				
+				var option1 = $document.createElement("option")
+				option1.setAttribute("value", "a")
+				select.appendChild(option1)
+				
+				var option2 = $document.createElement("option")
+				option2.setAttribute("value", "")
+				select.appendChild(option2)
+				
+				select.value = "c"
+				
+				o(select.value).equals("")
+				o(select.selectedIndex).equals(-1)
+			})
+			o("setting invalid selectedIndex yields a selectedIndex of -1 and value of empty string even when there's an option whose value is empty string", function() {
+				var select = $document.createElement("select")
+				
+				var option1 = $document.createElement("option")
+				option1.setAttribute("value", "a")
+				select.appendChild(option1)
+				
+				var option2 = $document.createElement("option")
+				option2.setAttribute("value", "")
+				select.appendChild(option2)
+				
+				select.selectedIndex = -2
+				
+				o(select.value).equals("")
+				o(select.selectedIndex).equals(-1)
+			})
 		})
 	})
 })