-
Notifications
You must be signed in to change notification settings - Fork 213
fix(replay): check if recorder needs to be started on opt in #2694
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
9a83fec to
4cde6a1
Compare
|
Size Change: +2.83 kB (+0.06%) Total Size: 5.11 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 files reviewed, no comments
| this.consent.optInOut(true) | ||
| this._sync_opt_out_with_persistence() | ||
|
|
||
| // Start queue after opting in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Re: line +3068]
i guess we're calling start twice now in this case
i think it's idempotent but 🙈
See this comment inline on Graphite.
pauldambra
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great tests
🚢

Problem
community question pointed out that session recording didnt start after an opt_in() call
this.sessionRecording?.startIfEnabledOrStop()https://posthog.slack.com/archives/C03B04XGLAZ/p1764766008265899
Changes
in
opt_in_capturing(), callstartIfEnabledOrStop()to kick off recording again once opted in.Release info Sub-libraries affected
Libraries affected
Checklist
If releasing new changes
pnpm changesetto generate a changeset file