Skip to content

Conversation

@daniel-graham-amplitude
Copy link
Collaborator

Summary

Checklist

  • Does your PR title have the correct title format?
  • Does your PR have a breaking change?:

@daniel-graham-amplitude daniel-graham-amplitude marked this pull request as draft July 16, 2025 22:45
@promptless
Copy link

promptless bot commented Jul 16, 2025

✅ No documentation updates required.

@daniel-graham-amplitude daniel-graham-amplitude marked this pull request as ready for review July 17, 2025 03:18
Copy link
Contributor

Copilot AI left a 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 NetworkObservers class with global networkObserver from analytics-core
  • Updates network event handling to use subscribe/unsubscribe pattern with NetworkEventCallback
  • 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(),

@promptless
Copy link

promptless bot commented Jul 17, 2025

✅ No documentation updates required.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants