Fix broken order of nodes during hydration #6283
Conversation
| @@ -22,7 +22,7 @@ export function append(target: Node, node: Node) { | |||
| if (is_hydrating) { | |||
| nodes_to_detach.delete(node); | |||
| } | |||
| if (node.parentNode !== target) { | |||
| if (node.parentNode !== target || node.parentNode !== null) { | |||
There was a problem hiding this comment.
hmm..reading from face value, this doesn't seems like a correct fix, there's maybe something wrong earlier in the hydration process / rather in the append method.
adding || node.parent !== null seems to defeat the purpose of doing node.parentNode !== target check
There was a problem hiding this comment.
Yes, that would defeat the check and so I agree the code shouldn't be written this way. It's also worth noting though that the check was introduced along with the bug in #6204 so we may not need to keep the check as is. If you remove this check and the one in insert to go back to how the code was originally then the tests will still pass. I don't think that either change would be the correct change though. The tests all pass with either change, which probably shows a shortcoming in the tests
There was a problem hiding this comment.
the alternate of this is
if (node.parentNode !== target || node.nextSibling !== null) {taking care the case where node parent is already correct, but the order is wrong
There was a problem hiding this comment.
In insert the following passes as well:
if (
node.parentNode !== target ||
(!anchor && node.nextSibling !== null) ||
(anchor && node.nextSibling !== anchor)
) {Definitely looks much more sensible than my initial attempt, although I don't know there still are other edge cases not caught by the tests. Would there be any good way to test the idempotence property more directly?
|
Thanks for the PR. The test was merged in #6290, so I'm going to close this. The fix probably isn't the right one according to the description in #6279 (comment) |
Fix #6279
Not sure exactly why this works but it does fix the issue for me. Need to add tests.