Skip to content

Conversation

@ryan953
Copy link
Member

@ryan953 ryan953 commented May 20, 2025

The original replay context is a big ball of logic and hacks. It's hard to use and mixes a lot of responsibilities.

A while ago I wrote some smaller context providers to break things up, and remove some old hacks that are no longer needed now, either because rrweb improvements, and because some of the problems were self-inflicted (like some perf hacks, and just doing things the wrong way around).

So this is the first PR to wrap players with the new providers, and to swap out a couple places that use the old context, for calls into the new context. What's apparent already within this PR is that the new context will push current time re-renders down the react tree. I need to followup and edit a bunch more places before this will be functional, but in the mean time the providers will be a no-op because the new feature-flag is guarding the interaction points.

@ryan953 ryan953 requested review from a team as code owners May 20, 2025 21:20
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label May 20, 2025
return (
<PlayerContainer data-test-id="player-container">
{replay?.hasProcessingErrors() ? (
<ReplayProcessingError processingErrors={replay.processingErrors()} />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is this handled now?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, good call out. i meant to comment

It's inside the <ReplayLoaderState> which is already in this file.

The last place that still needs a LoaderState added is the main replay details page; so that's not in this PR yet

@ryan953 ryan953 merged commit 2344c81 into master May 21, 2025
42 checks passed
@ryan953 ryan953 deleted the ryan953/replay-new-context branch May 21, 2025 14:17
@codecov
Copy link

codecov bot commented May 21, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #91982      +/-   ##
==========================================
- Coverage   86.50%   83.94%   -2.56%     
==========================================
  Files       10355    10355              
  Lines      587384   587382       -2     
  Branches    22586    22586              
==========================================
- Hits       508106   493098   -15008     
- Misses      78851    93857   +15006     
  Partials      427      427              

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

Labels

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