Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Duplicate keys is not reported in a clear way #409

Open
yminsky opened this issue Aug 31, 2016 · 6 comments
Open

Duplicate keys is not reported in a clear way #409

yminsky opened this issue Aug 31, 2016 · 6 comments

Comments

@yminsky
Copy link

yminsky commented Aug 31, 2016

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.

@bendrucker
Copy link

Duplicate keys as in duplicate keys in an object?

@jgaskins
Copy link

Issue is not reported in a clear way ;-)

@yminsky
Copy link
Author

yminsky commented Aug 31, 2016

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.

@jgaskins
Copy link

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

@yminsky
Copy link
Author

yminsky commented Sep 1, 2016

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

@yminsky
Copy link
Author

yminsky commented Sep 1, 2016

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants