Skip to content

fix: First record is checkout#1150

Closed
mydea wants to merge 2 commits intorrweb-io:masterfrom
getsentry:fn/first-record-checkout-rrweb
Closed

fix: First record is checkout#1150
mydea wants to merge 2 commits intorrweb-io:masterfrom
getsentry:fn/first-record-checkout-rrweb

Conversation

@mydea
Copy link
Copy Markdown
Contributor

@mydea mydea commented Feb 21, 2023

I am honestly not 100% sure if this is a bug, or somehow intended.
Currently, the initial full snapshot will never have isCheckout === true. If I understand the isCheckout flag correctly, I wonder why that is the case, and why this isn't true for the initial full snapshot? Maybe there is a good reason for that, or I am misunderstanding something, but IMHO it makes more sense to mark this as a checkout in that case?

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Feb 21, 2023

🦋 Changeset detected

Latest commit: 52d78e9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 7 packages
Name Type
rrweb Patch
rrweb-snapshot Patch
rrdom Patch
rrdom-nodejs Patch
rrweb-player Patch
@rrweb/types Patch
@rrweb/web-extension Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@YunFeng0817
Copy link
Copy Markdown
Member

I think this is not a bug. The checkout feature corresponds to two config options: checkoutEveryNth and checkoutEveryNms . The feature allows building full snapshots after the recorder starts. And the isCheckout flag is used to distinguish the first fullsnapshot and later checked-out snapshots.

@mydea
Copy link
Copy Markdown
Contributor Author

mydea commented Feb 22, 2023

Hmm OK, so the idea is that isCheckout is only ever set/triggered when checkoutEveryXXX is set, and other than this it is never set - I see. Then maybe we have to find a different way to handle this 🤔

@mydea mydea closed this Feb 22, 2023
@mydea mydea deleted the fn/first-record-checkout-rrweb branch February 22, 2023 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants