Race condition between native browser state and VDom

[Fresheyeball]

When hitting a checkbox, there is race condition where the native browser state will set the checkbox to checked: true and Snabbom fails to set checked: false. Here is a minimal complete example

var inBetween; // holds the intermediate vnode

const vnode1 = hh('div', {}, [ // vnode1 has the div with 2 checkboxes set to false
  hh('input', { on : { change : () => patch(inBetween,vnode2)}, // when the first checkbox is changed, we change to vnode2
                props : { checked : false, type : "checkbox" } }, []),
  hh('input', { props : { checked : false, type : "checkbox" } }, [])
])

const vnode2 = hh('div', {}, [
  hh('input', { props : { checked : false, type : "checkbox" } }, []) // vnode2 has the div containing 1 checkbox set to false
])

inBetween = patch(container, vnode1); // works as expected

Reproduction

You can see this bug occur live here: https://plnkr.co/edit/TyvcMBT0F3TQ5YDq

document.getElementsByTagName("input").length == 2
document.getElementsByTagName("input")[0].checked == false
document.getElementsByTagName("input")[1].checked == false

1. Click the first checkbox

Expected Result

document.getElementsByTagName("input").length == 1
document.getElementsByTagName("input")[0].checked == false

Actual Result

document.getElementsByTagName("input").length == 1
document.getElementsByTagName("input")[0].checked == true

[mreinstein]

I deleted my previous comment because I was investigating this late last night, with a fatigued 🧠 .

In looking at this again this morning I definitely think there is a bug here. I re-packaged the PR that @Fresheyeball submitted in #955

Mostly I just re-structured the logic in an attempt to make it more clear why the value and checked properties are handled a bit differently from the other props.

Something that I'm wondering though...is this issue more widespread? I would think any/all boolean properties on DOM elements would be similarly affected (disabled, required, open, hidden, autoplay, etc.)

@jvanbruegge @paldepind would still love another set of 👀 on this and your thoughts.

[paldepind]

I think the special casing on value and checked in #951 and #955 is a bit worrying. One might worry that there are other cases where this is needed (of course, the current code is already special casing on value – but perhaps that too was a mistake).

I'm wondering to what extent this is a bug. The diffing process is fundamentally about comparing the previous state of the virtual DOM with the current state of the virtual DOM and applying any differences to the DOM. With this in mind, it is not surprising that problems arise when the state of the DOM is modified both by the browser and by Snabbdom. The current fix for value (that #995 extends to checked), in a sense, changes the scope of the diffing process to also inspect the DOM.

React has a notion of an uncontrolled input and a controlled input and one can choose between the two by choosing between value and defaultValue. As far as I can see, our current behavior doesn't match either of them but is something in between. And there is no way to get the controlled behavior in Snabbdom as far as I can think.

Inspired by what React does, one could have a different subobject in props or a separate module for when one wants this semi-managed behavior. Then one would use that for value, checked, and perhaps other properties.

Related: #71 and #63.

[paldepind]

Also related #418 (thanks @iambumblehead).