-
Notifications
You must be signed in to change notification settings - Fork 175
feat: lazy load much more of session replay #2246
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
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Size Change: -78.7 kB (-1.5%) Total Size: 5.15 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.
Pull Request Overview
Implements lazy loading for the majority of session recording code, reducing bundle size by ~14% for customers who don't enable replay, and fixing a race condition between starting rrweb and receiving remote config.
- Consolidates the SessionRecording class into a lightweight wrapper that manages lazy loading
- Removes the SessionRecordingWrapper class which was used for testing
- Moves most session recording logic into a lazy-loaded extension
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
File | Description |
---|---|
packages/browser/terser-mangled-names.json | Updates mangled names list for refactored code |
packages/browser/src/types.ts | Deprecates preview flag that's no longer needed |
packages/browser/src/posthog-core.ts | Simplifies initialization to always use new lazy loading approach |
packages/browser/src/extensions/replay/sessionrecording.ts | Major refactor to lightweight wrapper managing lazy loading |
packages/browser/src/extensions/replay/sessionrecording-wrapper.ts | Removes wrapper class (entire file deleted) |
packages/browser/src/extensions/replay/sessionrecording-utils.ts | Removes buffer splitting utilities |
packages/browser/src/entrypoints/recorder.ts | Adds initialization function for lazy-loaded recording |
packages/browser/src/tests/extensions/replay/ | Updates tests to work with new lazy loading structure |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
packages/browser/src/__tests__/extensions/replay/sessionrecording.test.ts
Show resolved
Hide resolved
packages/browser/src/__tests__/extensions/replay/sessionrecording.test.ts
Show resolved
Hide resolved
testMatch: [ | ||
'**/__tests__/**/*.test.[jt]s?(x)', | ||
'**/?(*.)+test.[jt]s?(x)', | ||
'**/functional_tests/**/*.test.[jt]s?(x)', | ||
], |
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.
i had to do this because i extracted a utils.ts
file in the tests folder and then jest was unhappy because it was trying to run it as a test file
bfd672e
to
020e277
Compare
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.
Tested locally, a bit scared to deploy on Friday 🗄️
a lot of the session replay code predates (or at least overlapped with the introduction of) our lazy loading
so, even though we lazy load rrweb
a large chunk of the bundle is the code that interacts with rrweb
this PR moves as much as possible of that code into a lazy loaded extension
what does this change?
for customers that aren't enabling replay the bundle is ~14% smaller
for customers that are enabling replay we've removed the race between starting rrweb and receiving remoteconfig, this is probably ok, since remote config needs to be fast for flags, but does mean some customers might see replay start very slight later than today... but given we (want to) control what script to load via remote config and allow configuring masking via remote config this is probably desirable overall