Skip to content

Conversation

jerryzhou196
Copy link
Member

@jerryzhou196 jerryzhou196 commented Oct 9, 2025

Makes the refresh button only reload what's necessary instead of a full-page refresh.

before after
before after

requires #101462

jerryzhou196 and others added 11 commits October 6, 2025 15:32
Introduces two new hooks: useIsLive to determine if a replay is currently live and usePollReplayRecord to poll the backend for updates on the replay record.
The ReplayDetailsUserBadge component is updated to utilize these hooks, enhancing the live indicator functionality and allowing for automatic updates of replay data.
…state

Refactored the usePollReplayRecord hook to utilize useRef for tracking the updated state of the replay record. This change ensures that the updated state persists across renders without causing unnecessary re-renders. Additionally, clarified the pollInterval comment for better understanding.
…rBadge component

- addressed Ryans timeout comment
- addressed Billys discussion with useQuery
- addressed Billys > 0 for timeout
…ooks

- Updated time calculation in useIsLive to use a more readable format.
- Renamed replayRecord to polledReplayRecord in usePollReplayRecord for clarity.
- Removed unnecessary priority prop from RefreshButton in ReplayDetailsUserBadge component.
- Added priority prop to the RefreshButton in ReplayDetailsUserBadge component for improved button emphasis.
- Removed custom background and hover color styles from RefreshButton to streamline its appearance.
Removed primary priority from RefreshButton and added custom styles.
Copy link

linear bot commented Oct 9, 2025

@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Oct 9, 2025
…efetch in background + investigating memory leaks caused by console log capturing breadcrumbs via SDK
- Improved the refresh button logic to ensure proper query refetching and invalidation, now also triggering a summary request after refreshing.
@jerryzhou196 jerryzhou196 marked this pull request as ready for review October 14, 2025 19:22
@jerryzhou196 jerryzhou196 requested a review from a team as a code owner October 14, 2025 19:22
cursor[bot]

This comment was marked as outdated.

@billyvg billyvg changed the title feat(replay) - Background refetch (Billy's Edition) feat(replay) - Background refetch Oct 14, 2025
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Oct 14, 2025
Copy link
Contributor

🚨 Warning: This pull request contains Frontend and Backend changes!

It's discouraged to make changes to Sentry's Frontend and Backend in a single pull request. The Frontend and Backend are not atomically deployed. If the changes are interdependent of each other, they must be separated into two pull requests and be made forward or backwards compatible, such that the Backend or Frontend can be safely deployed independently.

Have questions? Please ask in the #discuss-dev-infra channel.

@jerryzhou196 jerryzhou196 force-pushed the replay-553-background-fetch-segment-react-query-refresh branch from ec1099a to c5da035 Compare October 14, 2025 21:19
Co-authored-by: ryan.albrecht <ryan.albrecht@sentry.io>
cursor[bot]

This comment was marked as outdated.

- Introduced a new event replay.details-refresh-clicked to track user interactions with the refresh button in the Replay Details view.
- Updated the ReplayDetailsUserBadge component to call the tracking function upon refresh action.
@jerryzhou196
Copy link
Member Author

@ryan953 I saw you simplified the showRefreshButton logic.

The reason why we had this convoluted usage of showRefreshButton state is so polling would stop after displaying the refreshButton. Now, it'll continue polling regardless, but if you think the overhead of that is minimal, I'm happy to use this since it's def more readable.

Looping in @billyvg as well.

@ryan953
Copy link
Member

ryan953 commented Oct 15, 2025

@ryan953 I saw you simplified the showRefreshButton logic.

The reason why we had this convoluted usage of showRefreshButton state is so polling would stop after displaying the refreshButton. Now, it'll continue polling regardless, but if you think the overhead of that is minimal, I'm happy to use this since it's def more readable.

Looping in @billyvg as well.

I was trying to demonstrate that the logic can be put into a var instead of state and typed ... to indicate that if we need more conditions then that's fine to add.

For example, the const isUpdated = useRef(false); might be something to bring back as an extra condition to help stop polling. useRef is nice because it doesn't cause a re-render when you set it. Before there was state being set, and the potential for a useEffect, which would cause an extra re-render and was unnecessary.

We should try to stop the polling if we can, so I guess there's still some details that you'll be able to work out.

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

Labels

Scope: Backend Automatically applied to PRs that change backend components Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants