Matt-Esch/virtual-dom

Duplicate keys is not reported in a clear way

yminsky opened this issue · 6 comments

Instead, it just leads to incorrect patches being created, which causes things to blow up farther down the line at some unspecified point in time.

It would be nice if duplicating keys would lead to an exception that explicitly flagged this problem.

Even better would be for the diffing to be done in a correct, if less efficient way, in that case.

Duplicate keys as in duplicate keys in an object?

Issue is not reported in a clear way ;-)

The problematic case is where you have two different vnodes with the same key. This causes the diffing algorithm to do the wrong thing (dropping nodes that shouldn't be dropped), silently.

There's still a bit of ambiguity here. Can you show an example?

Sure. I have some examples I can put here in the morning. Just to be clear, though, the issue comes up specifically when you reuse the same key in multiple vnodes that have the same parent. By the key, I simply mean the argument called "key" in the constructor, here:

https://github.com/Matt-Esch/virtual-dom/blob/master/vnode/vnode.js#L12

Here's a concrete example.

vdom1: https://gist.github.com/yminsky/e01531a14bc96e9b0bb474cc07a43aa6
vdom2: https://gist.github.com/yminsky/3933efc5faf4eef314ecdeea5c0e89bf
patch: https://gist.github.com/yminsky/b9d6dc4e5e4ab67e8b9c2e6bfe728a14

The patch is the patch between vdom1 and vdom2, but setting the DOM to vdom1 and then applying the patch yields a different state than setting it to vdom2 directly.

I got these patches by dumping the JSON from the browser.