Mithril.js: Re-using hyperscript nodes leads to wrong updates on m.redraw()

[dvdkhlng]

Mithril Version: next

Expected Behavior

Looking at the docs, I think it should be ok to keep around copies of nodes returned by m() and re-arrange them (unmodified) into a new parent when view() is called. Note that I never try to have the same node referenced more than once within one node tree.

Current Behavior

However, in practice this leads to strange updating bugs, where the DOM rendered by Mithril diverges from the nodes returned by view().

Steps to Reproduce

Minimal example of the observed unexpected behaviour is here:

https://jsfiddle.net/97qthkp1/1/

Pressing "update" adds new nodes, and at more than 3 nodes we throw away the oldest node when appending a new node.

However, running this in the latest Firefox, this fails to properly re-order the node list after the 4th press of "update", only updating the first node and still showing the old nodes that are already gone.

Context

This issue happens in some experimental code that generates and updates potentially large SVG charts. I'm new to javascript and mithril, so maybe I didn't get some obvious fact that explains this all.

Workaround

Always re-creating the m() nodes with the same parameters (instead of keeping around the object reference) works around the issue, but looks pretty suboptimal to me (suspecting possible interference with the tree diff'ing).

Your Environment

• Browser Name and version: Firefox 67.0.4 (64-bit)
• Operating System: Destop / Ubuntu

[osban]

@dvdkhlng did you try adding a key attribute, like this.queue.push(m("p", {key: Date.now()}, this.count.toString()))?

[dvdkhlng]

@osban: you are right, adding a key attribute fixes (or works around) the issue:
https://jsfiddle.net/q0Layhw8/1/

Does this have any effect on the validity of the bug report? I'm a little lost, the Mithril manual does not have any example code that keeps around references to nodes to be returned in view().

[isiahmeadows]

@dvdkhlng Not a bug, but undefined behavior. When you're mutating this.queue, that's being returned as a literal fragment vnode, roughly equivalent to m.fragment({}, this.queue). Mithril uses this.queue directly as the children of interest, and mutating that counts as mutating a mutable vnode. To clarify how the docs apply here:

Vnodes are supposed to represent the state of the DOM at a certain point in time. Mithril's rendering engine assumes a reused vnode is unchanged, so modifying a vnode that was used in a previous render will result in undefined behavior.

(Emphasis here, not in docs)

If you really want this behavior, add keys to all the vnodes in question and then return this.queue.slice() instead. That way, Mithril will at least know identities moved around. But in either case, don't do this. Just generate the vnodes as you need them and use keys as necessary.

[osban]

@dvdkhlng If possible in your case, you could do something like this, so instead of working with vnodes, work with data.

[dvdkhlng]

@isiahmeadows : modifying queue after I gave it to Mithril was something that I totally overlooked. Ouch. Javascript semantics take some time to getting used to...

@osban, yes will move to working on data instead of nodes.

thanks for your helpful comments.

[isiahmeadows]

@dvdkhlng

modifying queue after I gave it to Mithril was something that I totally overlooked. Ouch. Javascript semantics take some time to getting used to...

Mithril is rather unique among vdom implementations in that it mutates the vnodes it receives. So don't worry, especially if you're coming from a different virtual DOM framework like React. (I didn't make that design choice, BTW.)