rrweb-snapshot icon indicating copy to clipboard operation
rrweb-snapshot copied to clipboard

Reliably generate mutations while the snapshot is iterating

Open eoghanmurray opened this issue 4 years ago • 1 comments
trafficstars

Reliably generate mutations while the snapshot is iterating in order to test rarely occurring problems. Have added what I believe should be the correct parsing, on the assumption that the DOM changes will subsequently be correctly picked up by mutation events.

  • The difference between test cases a & b demonstrate different outcomes because of the freezing of childNodes in for (const childN of Array.from(n.childNodes)) — I'm not sure if there's a better way to iterate over child nodes (I imagine this freezing was intentional)
  • Test case d takes this further to show that it should not be possible to iterate to the #d3 element
  • The final e test case is the important one as it demonstrates that the snapshot can produce duplicate nodes, in this case #a1 appears twice in the output

I authored https://github.com/rrweb-io/rrweb-snapshot/pull/60 assuming there was something similar to the e test case going on. But maybe a better solution would be to ensure all __sn attributes are wiped before the start of a snapshot, and ignore any nodes that already have a __sn assigned if we encounter them in the DOM walk, as presumably they have a lower id from earlier in the walk.

Node: The code modifications in snapshot.ts to get these tests to work would need to be removed or somehow only targetted to a special test build.

I'd like to get further input into this before proceeding on fixes for the test case failures I've created in this pull request!

eoghanmurray avatar Feb 01 '21 21:02 eoghanmurray

@Yuyz0112 Just to note that it is intentional that the tests are failing for this pull request (in case you are avoiding it for that reason).

I also need some good ideas as to how to target the bit of code which does the eval in https://github.com/rrweb-io/rrweb-snapshot/pull/61/commits/d6b57fcfa09d9cba4e4aa203c42ae8cddc4eb055#diff-75923536d00ea7b9bdbd2a5196a698720893adb4993f502c3d777ca9b51a4df2 so that it only executes in testing?

eoghanmurray avatar Jun 15 '21 13:06 eoghanmurray