-
Notifications
You must be signed in to change notification settings - Fork 54
refactor: use global networkObserver in session replay #1213
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?
refactor: use global networkObserver in session replay #1213
Conversation
|
✅ No documentation updates required. |
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
This PR refactors the session replay package to use a global network observer from the analytics-core package instead of a local implementation. The purpose is to consolidate network observation functionality into a shared utility.
- Replaces local
NetworkObserversclass with globalnetworkObserverfrom analytics-core - Updates network event handling to use
subscribe/unsubscribepattern withNetworkEventCallback - Removes local network observer implementation and associated tests
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/session-replay-browser/src/session-replay.ts | Updates to use global networkObserver with subscribe/unsubscribe pattern |
| packages/session-replay-browser/src/observers.ts | Completely removed local NetworkObservers implementation |
| packages/session-replay-browser/test/session-replay.test.ts | Updates tests to mock global networkObserver instead of local implementation |
| packages/session-replay-browser/test/observers.test.ts | Removes all tests for local NetworkObservers class |
Comments suppressed due to low confidence (2)
packages/session-replay-browser/test/session-replay.test.ts:1924
- The test is using Date.now() for both startTime and endTime, which may not properly test time-based functionality. Consider using jest.useFakeTimers() or mocking Date.now() to control the time values in tests.
startTime: Date.now(),
packages/session-replay-browser/test/session-replay.test.ts:1925
- The test is using Date.now() for both startTime and endTime, which may not properly test time-based functionality. Consider using jest.useFakeTimers() or mocking Date.now() to control the time values in tests.
endTime: Date.now(),
|
✅ No documentation updates required. |
… AMP-125616/refactor-sr-to-use-singleton-network
Summary
Checklist