search

elm / virtual-dom

The foundation of HTML and SVG in Elm.
https://package.elm-lang.org/packages/elm/virtual-dom/latest
BSD 3-Clause "New" or "Revised" License
209 stars 80 forks source link

Fix which `key` is used in swapping nodes in diffKeyedKids #154

Closed MarcoPolo closed 2 years ago

MarcoPolo commented 5 years ago

I think I've found a semantic bug in _VirtualDom_diffKeyedKids function. It doesn't seem to affect anything, so it's probably fine as is, but I thought this would still be interesting.

Consider this: x is A B C D y is A C B D

B is a new match and the xNode and yNextNode C is an old match and the yNode and xNextNode

yNode == xNode

We first diff x's B and y's B _VirtualDom_diffHelp(xNode, yNextNode, localPatches, index); Then we insert y's C (yNode) Then we remove the x's C (xNextNode)

When we do the insert/remove of C we should be using yKey (or xNextKey) because that is what C's key is.

With all that being said,

This is only a semantic bug. The code works fine because since xKey is used in both places it becomes a Move like it would otherwise.

TysonMN commented 4 years ago

Ah, yes. I agree.

To repeat, semantically the key in both calls should be yKey. However, there is no bug because it suffices for the same unique identifier to be passed into both function calls, which is the case.