Skip to content
This repository was archived by the owner on Aug 30, 2023. It is now read-only.

ref: Refactor the rrweb record handler#81

Merged
billyvg merged 3 commits intomainfrom
ref/change-recording-rrweb-events
Jun 3, 2022
Merged

ref: Refactor the rrweb record handler#81
billyvg merged 3 commits intomainfrom
ref/change-recording-rrweb-events

Conversation

@billyvg
Copy link
Copy Markdown
Member

@billyvg billyvg commented Jun 2, 2022

Refactor the batching logic into the method: addReplayEvent. This will help us apply the same logic for our core SDK instrumentation handlers.

Refactor this handler so that we can re-use `addReplayEvent`. This will help us apply the same logic for our core SDK instrumentation handlers.
@billyvg billyvg force-pushed the ref/change-recording-rrweb-events branch from c4db660 to 00d7d67 Compare June 2, 2022 19:19
@billyvg billyvg marked this pull request as ready for review June 2, 2022 19:19
@billyvg billyvg requested a review from JoshFerge June 2, 2022 19:20
Comment on lines +312 to +316
// Suppress console.errors
jest.spyOn(console, 'error').mockImplementation(jest.fn());
const mockConsole = console.error as jest.MockedFunction<
typeof console.error
>;
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed this because otherwise console.error messages were being dumped into terminal when running tests. cc @Danondso

Comment on lines +244 to +247
if (cb() === true) {
this.finishReplayEvent();
return;
}
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really the only thing that changed

Copy link
Copy Markdown
Member

@JoshFerge JoshFerge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

much cleaner great stuff 👍🏼

@billyvg billyvg merged commit 27c285b into main Jun 3, 2022
@billyvg billyvg deleted the ref/change-recording-rrweb-events branch June 3, 2022 13:53
mydea pushed a commit to getsentry/sentry-javascript that referenced this pull request Nov 23, 2022
Refactor this handler so that we can re-use `addReplayEvent`. This will help us apply the same logic for our core SDK instrumentation handlers.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants