ref(replay): Handle checkouts more explicitly #7321
Conversation
| "id": 15, | ||
| "x": 157, | ||
| "y": 90 | ||
| }, |
There was a problem hiding this comment.
I have no idea why this is suddenly ordered differently... It seems to be consistently so now. Not sure @cmanallen / @billyvg if you know, does this make any difference, or is the order of records in this incremental snapshot array not important? I don't really see timestamps here, but not sure...
There was a problem hiding this comment.
Hmm not sure, also interesting it doesn't have a type either
| "rel": "icon", | ||
| "type": "image/png", | ||
| "href": "file://assets/icon/favicon.png" | ||
| "rr_width": "[0-50]px", |
There was a problem hiding this comment.
This is unrelated but wasn't updated and fails locally.
size-limit report 📦
|
mydea
force-pushed
the
fn/replay-handle-checkout
branch
from
4bbec5d to
ee92a14
Compare
Lms24
left a comment
Lms24
left a comment
There was a problem hiding this comment.
LGTM - also thanks for extracting out the function (I think we can take the slight bundle size hit over readability/maintainability)
|
|
||
| const reqPromise0 = waitForReplayRequest(page, 0); | ||
| const reqPromise1 = waitForReplayRequest(page, 1); | ||
| const reqPromise2 = waitForReplayRequest(page, 2); |
There was a problem hiding this comment.
(no action required)
Good change, let's just always keep these at the top of tests for consistency going forward
billyvg
left a comment
billyvg
left a comment
There was a problem hiding this comment.
Makes sense to me, I think the missing isCheckout on the initial payload is why I had originally used the FullSnapshot type.
| "id": 15, | ||
| "x": 157, | ||
| "y": 90 | ||
| }, |
There was a problem hiding this comment.
Hmm not sure, also interesting it doesn't have a type either
mydea
force-pushed
the
fn/replay-handle-checkout
branch
3 times, most recently
from
c6a8c24 to
cdb2b17
Compare
mydea
force-pushed
the
fn/replay-handle-checkout
branch
from
cdb2b17 to
c6cef1b
Compare
We used to look at
event.type === 2for replay events to infer if something is a checkout or not.This is a bit hacky, as type === 2 actually just means it is a full snapshot, which may or may not be due to a checkout.
This makes it hard/impossible to do any future changes where we may want to do a full snapshot in other scenarios than in error mode, e.g. if we want to do a full snapshot to avoid large mutations.
This PR refactors this a bit to really just look at
isCheckout, plus also handle the very first rrweb event as a checkout (because we rely on this a bit).This replaces #7237