Skip to content

ref(replay): Extract sticky session handling #6315

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

Merged
merged 1 commit into from
Nov 30, 2022
Merged

Conversation

mydea
Copy link
Member

@mydea mydea commented Nov 28, 2022

Currently, a Session in replay receives stickySession as argument, and conditionally saves itself in its setters.

IMHO this responsibility is better placed outside of the session class. This also allows us to streamline the Session class quite a lot.

@mydea mydea added the Package: replay Issues related to the Sentry Replay SDK label Nov 28, 2022
@mydea mydea requested a review from Lms24 November 28, 2022 15:55
@mydea mydea self-assigned this Nov 28, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Nov 28, 2022

size-limit report 📦

Path Size
@sentry/browser - ES5 CDN Bundle (gzipped + minified) 19.57 KB (-0.02% 🔽)
@sentry/browser - ES5 CDN Bundle (minified) 60.59 KB (0%)
@sentry/browser - ES6 CDN Bundle (gzipped + minified) 18.36 KB (+0.01% 🔺)
@sentry/browser - ES6 CDN Bundle (minified) 54.15 KB (0%)
@sentry/browser - Webpack (gzipped + minified) 20.12 KB (0%)
@sentry/browser - Webpack (minified) 65.75 KB (0%)
@sentry/react - Webpack (gzipped + minified) 20.14 KB (0%)
@sentry/nextjs Client - Webpack (gzipped + minified) 45.99 KB (0%)
@sentry/browser + @sentry/tracing - ES5 CDN Bundle (gzipped + minified) 26.44 KB (-0.01% 🔽)
@sentry/browser + @sentry/tracing - ES6 CDN Bundle (gzipped + minified) 24.88 KB (+0.02% 🔺)
@sentry/replay index.js 62 KB (-0.13% 🔽)
@sentry/replay - Webpack (gzipped + minified) 62.01 KB (-0.13% 🔽)
@sentry/replay - Webpack (minified) 199.35 KB (-0.25% 🔽)

@mydea mydea force-pushed the fn/replay-explicit-sticky branch from 50b59eb to 2f6c929 Compare November 29, 2022 14:23
Copy link
Member

@Lms24 Lms24 left a comment

Choose a reason for hiding this comment

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

I think it's a good refactor because a) the actual saveSession logic wasn't in the class before so IMO it makes sense to separate it completely and b) this gets rid of the circular dependency we had between session.ts and saveSession.ts.

@mydea mydea force-pushed the fn/replay-explicit-sticky branch from 2f6c929 to 22c79cf Compare November 30, 2022 14:27
@mydea mydea merged commit d7e72fb into master Nov 30, 2022
@mydea mydea deleted the fn/replay-explicit-sticky branch November 30, 2022 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: replay Issues related to the Sentry Replay SDK
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants