Skip to content

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.

3 participants