Matt-Esch / virtual-dom

A Virtual DOM and diffing algorithm
MIT License
11.67k stars 777 forks source link

Virtualize and Virtual-dom: class lost patching a virtualized element. #280

Open cristiano-belloni opened 9 years ago

cristiano-belloni commented 9 years ago

Hi, I'm trying to write a simple proof of concept. I would like to virtualize an existing DOM node, generate a new VTree with h, then diff the two and patch the DOM node.

What I do is:

var vtree = h('div', null, [
                h('button', {onclick: onClick}, 'Modify')
              , h('div.vdom-content', null, [])
              ])
              , rootNode = createElement(vtree)

              node.appendChild(rootNode)

and then, when the button is clicked, I select the .vdom-content element and pass it to the view function:

function render() {
        return h('div.vdom-content', null, [])
      }

      function view(node) {
        var oldVtree = virtualize(node)
            , newVtree = render()
            , patches = diff(oldVtree, newVtree)

        patch(node, patches)
      }

This result in the vdom-content class in my patch being removed, which is quite surprising. The only difference between the oldVtree and the newVtree is that oldVtree, generated by virtualize, has the properties.attributes object, and newVtree does not:

attributes: { class: "vdom-content" }

They both have properties.className, set as vdom-content.

The calculated patches object looks like this:

vnode

Is this expected? Obviously I would like to be able to correctly diff between virtualizd nodes and virtual nodes.

This issue is also posted on the vdom-virtualize board (https://github.com/marcelklehr/vdom-virtualize/issues/31)

marcelklehr commented 9 years ago

Why don't you create a requirebin? that should make it easier to grasp or find mistakes.

bitinn commented 9 years ago

My best guess given current information, you forget to updated your rootNode reference somehow, ie. rootNode = patch(...), I ran into similar issues while doing the same thing.

When in doubt, you can also use https://github.com/bitinn/vdom-parser to double-check the virtualization is correct.

martintietz commented 9 years ago

Just want to mention, that I ran into the same issue. After replacing vdom-virtualize with vdom-parser everything is working fine again, so I guess that it has to be an issue inside vdom-virtualize. I compared the Vtrees generated by both of them and the only difference I could spot was that vdom-virtualize creates a property properties.attributes for each vdom-element and vdom-parser creates no such property.

marcelklehr commented 8 years ago

I think, the problem is that applyProperties treats attributes like any other property. Thus className is set, but removeAttribute is called afterwards, effectively overriding className. A fix would be to apply changes to attributes first and only then apply properties. Scratch that, the problem is that diff finds no change in className, but a removed attribute. Thus patch removes the attribute and the browser overrides className. I don't know how to fix this in virtual-dom. I can create a blacklist to take handling of all those attributes that are handled by properties out of the virtualizer.