Skip to content
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

feat(replay): Calculate hydration diff timestamps based on related hydration breadcrumbs #73095

Merged
merged 4 commits into from
Jun 24, 2024

Conversation

ryan953
Copy link
Member

@ryan953 ryan953 commented Jun 20, 2024

This is the 2nd try at #72561
The original was reverted in 986296c because of some errors that popped up:

The difference now is that I've improved the types to include data.mutations.next. The real but though was that before, in breadcrumbItem.tsx, we were doing the left/right timestamp math only for crumbs that have that mutations.next field...

That problem is fixed now because we're checking the crumb type first, then defer to the new <CrumbHydrationButton> which will get the offsets and render all at once.
Without that fix, we were basically trying to get the left/right offsets for any breadcrumb type, which would easily explode.

Related to #70199

@ryan953 ryan953 requested a review from a team as a code owner June 20, 2024 22:57
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Jun 20, 2024
@ryan953 ryan953 changed the title feat(replay): Calculate hydration diff timestamps based on related hy… feat(replay): Calculate hydration diff timestamps based on related hy dration breadcrumbs Jun 20, 2024
Copy link

codecov bot commented Jun 20, 2024

Bundle Report

Changes will decrease total bundle size by 1.33kB ⬇️

Bundle name Size Change
app-webpack-bundle-array-push 27.3MB 1.33kB ⬇️

Copy link

codecov bot commented Jun 20, 2024

Codecov Report

Attention: Patch coverage is 60.00000% with 10 lines in your changes missing coverage. Please review.

Project coverage is 77.98%. Comparing base (5d4a152) to head (e99821a).
Report is 3 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #73095      +/-   ##
==========================================
+ Coverage   70.83%   77.98%   +7.15%     
==========================================
  Files        6629     6619      -10     
  Lines      295890   295061     -829     
  Branches    50965    50830     -135     
==========================================
+ Hits       209585   230100   +20515     
+ Misses      79293    58653   -20640     
+ Partials     7012     6308     -704     
Files Coverage Δ
...replays/breadcrumbs/openReplayComparisonButton.tsx 18.18% <ø> (ø)
...ents/replays/breadcrumbs/replayComparisonModal.tsx 0.00% <ø> (ø)
static/app/utils/replays/replayReader.tsx 84.12% <100.00%> (+25.08%) ⬆️
static/app/utils/replays/types.tsx 72.09% <100.00%> (+45.26%) ⬆️
...ts/events/eventHydrationDiff/replayDiffContent.tsx 0.00% <0.00%> (ø)
static/app/components/replays/replayDiff.tsx 0.00% <0.00%> (ø)
.../components/replays/breadcrumbs/breadcrumbItem.tsx 49.23% <0.00%> (+33.10%) ⬆️
static/app/utils/replays/getDiffTimestamps.tsx 75.00% <75.00%> (ø)

... and 1405 files with indirect coverage changes

@ryan953 ryan953 changed the title feat(replay): Calculate hydration diff timestamps based on related hy dration breadcrumbs feat(replay): Calculate hydration diff timestamps based on related hydration breadcrumbs Jun 21, 2024
@ryan953 ryan953 enabled auto-merge (squash) June 24, 2024 18:54
@ryan953 ryan953 merged commit 50fdd0e into master Jun 24, 2024
42 checks passed
@ryan953 ryan953 deleted the ryan953/hydration-error-diff-offsets-v2 branch June 24, 2024 19:02
Copy link

sentry-io bot commented Jul 1, 2024

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ TypeError: Cannot read properties of undefined (reading 'next') getReplayDiffOffsetsFromFrame(getDiffTimestamps... View Issue
  • ‼️ TypeError: Cannot read properties of undefined (reading 'next') getReplayDiffOffsetsFromFrame(getDiffTimestamps... View Issue
  • ‼️ TypeError: Cannot read properties of undefined (reading 'next') getReplayDiffOffsetsFromFrame(getDiffTimestamps... View Issue

Did you find this useful? React with a 👍 or 👎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Scope: Frontend Automatically applied to PRs that change frontend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants