Reusing immutable nodes in the same render produces incorrect DOM

[JAForbes]

💡 Note this is a different issue to #2441 where the list was mutated, this bug occurs when there's no mutation at all.

Mithril version:

Version 2.0.3

Browser and OS:

Chrome / Windows

Project:

flems

Expected Behavior

Reusing a several times node within a render should not cause mithril to use the node from the previous render.

Current Behavior

Mithril uses the node from the previous render except for the last usage of the vnode.

Context

The documentation specifies:

Avoid memoizing mutable vnodes`

But this is not a mutation of a node, just a reuse. This behaviour means certain rendering patterns with streams become dangerous, that were (prior to v1 at least) previously safe.

[dead-claudia]

It's a docs bug. The heading needs to change to something more specific, like "Avoid reusing vnodes in different positions".

[JAForbes]

Breaking referential transparency is unsurprising behaviour, updating the docs to describe the surprising behavior will work. But would having two caches be a significant amount of work?

If we update the cache logic, the docs can stay the same and we've got unsurprising behaviour that dovetails with streams to give us performance benefits that other frameworks don't have.

It's unintuitive that vnode's aren't like any other value.

[dead-claudia]

@JAForbes The greater issue is that of vnode.dom - you can't have the same underlying node in two different positions in the DOM simultaneously. (It'd be much easier if this weren't the case IMHO.)

[JAForbes]

Ah I see, that's a shame. I think that exact justification would be really helpful in the documentation. Thanks @isiahmeadows for walking me through that.

[dead-claudia]

Reopening because the docs still need fixed.

[JAForbes]

I haven't made a docs contribution yet, I'd be happy to give it a go though.

[17dec]

In case anyone's running into a "Node.insertBefore: Child to insert before is not a child of this node" error, you're likely hitting this issue.

Was about to report this as a behavioral bug, but turns out I was just using it wrong. Even made a small test case :)

let a = ['a'];
m.mount(document.body, {
    view: function() {
        const X = m('b', 'X');
        return [
            m('button[type=button]', { onclick: () => a.push('a') }, a, X), X
        ]
    }
})