Skip to content

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
@codecov
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 ⬇️

@codecov
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
@ryan953 ryan953 deleted the ryan953/hydration-error-diff-offsets-v2 branch June 24, 2024 19:02
@sentry-io
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 👎

@github-actions github-actions bot locked and limited conversation to collaborators Jul 17, 2024
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