Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: isCheckout is not included in fullsnapshot event #1141

Merged
merged 2 commits into from
Feb 17, 2023
Merged

Conversation

YunFeng0817
Copy link
Member

bug

When taking a fullsnapshot with the option isCheckout, the property is not included in the fullsnapshot event. I guess this property is important for a lot of use cases.

@changeset-bot
Copy link

changeset-bot bot commented Feb 17, 2023

🦋 Changeset detected

Latest commit: f918c91

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

@Juice10 Juice10 merged commit 3416c3a into master Feb 17, 2023
@Juice10 Juice10 deleted the fix-isCheckout branch February 17, 2023 07:51
billyvg added a commit to getsentry/rrweb that referenced this pull request Dec 8, 2023
billyvg added a commit to getsentry/rrweb that referenced this pull request Dec 11, 2023
…io#1141)" (#139)

This reverts commit 3416c3a.


From [my comment
here](rrweb-io#1242 (comment))

> I only used translate on the OP but if I understand correctly, when we
call `takeFullSnapshot(true)`, we will receive two distinct events with
the `isCheckout` arg as `true`. Looking at the blame this is due to
rrweb-io#1141.

> If you use `checkoutEvery...` and rely on `isCheckout` to reset
existing events, this means that the [`Meta`
event](https://github.com/rrweb-io/rrweb/blob/master/packages/rrweb/src/record/index.ts#L352-L362)
will be lost because [the
`FullSnapshot`](https://github.com/rrweb-io/rrweb/blob/master/packages/rrweb/src/record/index.ts#L408-L417)
occurs afterwards, and `isCheckout` is true for both events. Losing the
`Meta` event means that the replayer will be unable to set its
dimensions, making the replay look broken.
billyvg added a commit to getsentry/rrweb that referenced this pull request Apr 26, 2024
…io#1141)" (#139)

This reverts commit 3416c3a.

From [my comment
here](rrweb-io#1242 (comment))

> I only used translate on the OP but if I understand correctly, when we
call `takeFullSnapshot(true)`, we will receive two distinct events with
the `isCheckout` arg as `true`. Looking at the blame this is due to

> If you use `checkoutEvery...` and rely on `isCheckout` to reset
existing events, this means that the [`Meta`
event](https://github.com/rrweb-io/rrweb/blob/master/packages/rrweb/src/record/index.ts#L352-L362)
will be lost because [the
`FullSnapshot`](https://github.com/rrweb-io/rrweb/blob/master/packages/rrweb/src/record/index.ts#L408-L417)
occurs afterwards, and `isCheckout` is true for both events. Losing the
`Meta` event means that the replayer will be unable to set its
dimensions, making the replay look broken.
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