fix: Failed to execute insertBefore on Node

[wfk007]

background

@omairnabiel encountered this runtime error a month ago after upgrade to 2.0.0.alpha.2 and reported at slack.
I encountered this error nowadays as well which totally block replay.

how it happens

In image below:

• the red block refer to FullSnapshot
• the green block refer to IncrementalSnapshot

After initialized, seek after the second fullSnapshot(the two fullSnapshot with different structure).

directly cause by

This error is directly caused by: insert a textNode into another textNode

textNode.insertBefore(newTextNode, null)

when I dive into createOrGetNode I find that the domMirror and rrnodeMirror returned different node type with the same nodeId

• domMirror returned a TextNode(causing that insert a textNode into another textNode)
• rrnodeMirror returned a ElementNode

So, i guess whether domMirror is outdated in some unknown reason

deep in

Then I combed the flow of replay:

1. filter neededEvents
2. filter syncEvents and asyncEvents based on neededEvents
3. asyncEvents add to Timer by addAction and invoked by Timer schedule(requestAnimationFrame) based on timestamp
4. syncEvents => applyEventsSynchronously => applyIncremental sync => applyMutation sync
5. buildFromDom and usingVirtualDom = true
6. during applyEventsSynchronously, ALL IncrementalSnapshot will apply to virtualDom NOT the realDOM
7. after applyEventsSynchronously, diff algorithm invoked => realDOM updated => virtualDom destroyed
8. all subsequent IncrementalSnapshot will directly update the realDOM

I find that during applyEventsSynchronously, if there is a fullSnapshot event, it will update this.mirror based on new fullSnapshot, but not rebuild it after this.mirror cleared

before rebuild a new fullSnapshot:

number of nodes in event.data.node

after rebuild a new fullSnapshot:

I also refer to [email protected]: in replay and rebuild, and you will get a clean idNodeMap every time rebuild

[Juice10]

Thank you for the thorough explanation, this must have been super time consuming to figure out, well done!
It would really be great if we could get a test that reproduces this failing state, that is fixed by the change you did. That way this issue will be solved forever and we won't have any regressions.

[wfk007]

Thank you for the thorough explanation, this must have been super time consuming to figure out, well done! It would really be great if we could get a test that reproduces this failing state, that is fixed by the change you did. That way this issue will be solved forever and we won't have any regressions.

ok, good idea

[wfk007]

Thank you for the thorough explanation, this must have been super time consuming to figure out, well done! It would really be great if we could get a test that reproduces this failing state, that is fixed by the change you did. That way this issue will be solved forever and we won't have any regressions.

@Juice10 Sorry for late!
I try to write a minimal reproducible code to generate events for test, but it keeps failing!
Additionally, I find some details in events of bad case:

generally speaking, id of node in childNodes should go after it in parentNode when taking fullSnapshot.
Id 65005 should be 8 in right case, and there are several wired id in events of bad case.
I event not sure how it really happened!

If new fullSnapshot are rebuild based on mirror with such wired ids. The mirror will be mistaken.
In case below:

• temp2 refer to domMirror
• temp1 refer to a textNode with id 82

[omairnabiel]

Is this PR ready to be merged?

[YunFeng0817]

Thanks for your great fix and thorough explanation!