Repository navigation
perf(metrics): stop re-rendering unchanged metric panels on URL changes - #126977
Open
JoshuaKGoldberg wants to merge 16 commits into
Open
JoshuaKGoldberg wants to merge 16 commits into
JoshuaKGoldberg wants to merge 16 commits into
Conversation
Expanding or collapsing a metric writes to the URL, which re-rendered every sample table row in every panel because rows read the location for their trace links. Memoize the rows and pass them a stable, metric-param-stripped location instead. Fixes LOGS-1035
github-actions
Bot
added
the
Scope: Frontend
JoshuaKGoldberg
changed the title
BaseExploreFieldRenderer read the location and route for its links, so every Explore table cell re-rendered its action menu, buttons, and tooltip on any URL change. A child component now reads them for the cell value alone. Refs LOGS-1035
…L changes The button tracking hooks returned a new function on every render, so each navigation changed the tracking context and re-rendered every Button and Link. The starred Explore queries list also rebuilt on any query param change, though it only reads the query id and pathname. Refs LOGS-1035
This reverts commit f412c68. The cells kept re-rendering through the per-panel query params context, so moving the location read into a child had no measurable effect. Refs LOGS-1035
Its links subscribe to the location themselves, so they kept re-rendering and the memo saved almost nothing. Refs LOGS-1035
… changes Every URL change re-decoded every metric query and rebuilt each row's mutators, so every panel's query params context changed and re-rendered its charts, toolbars, and table cells. Decoded queries are now reused for unchanged URL values, and each row gets stable mutators. Refs LOGS-1035
Restores f412c68. Now that unchanged metric panels keep stable query params, the location read was the last thing re-rendering each cell's action menu, buttons, and tooltip. Refs LOGS-1035
JoshuaKGoldberg
changed the title
…035-reduce-lag-when-expanding-or-collapsing-metrics # Conflicts: # static/app/views/explore/metrics/metricInfoTabs/metricsSamplesTableRow.tsx
Member
Author
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
…ut effects The stable row mutators read rows that a layout effect updated, but descendant layout effects run first. A group by cleanup in the same commit as a query change could write stale rows to the URL and revert that change. An insertion effect updates them before any layout effect runs. Refs LOGS-1035
…anels Panels with identical URL values received the same decoded query objects, so an in-place change to one would leak into the other. The cache is now keyed by panel position too. Also adds a regression test for the stale mutator fix. Refs LOGS-1035
Refs LOGS-1035
Member
Author
|
@cursor review |
Contributor
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 55ecb2c. Configure here.
JoshuaKGoldberg
marked this pull request as ready for review
October 14, 2026 14:20
nsdeschenes
approved these changes
Oct 19, 2026
nsdeschenes
left a comment
nsdeschenes
left a comment
Contributor
There was a problem hiding this comment.
LGTM ... tho i had to read up on useInsertionEffect to refresh my memory.
…test ref The tracker only needs the parameterized route path, which is a string that stays equal across query changes, so it can be a plain useCallback dependency.
Keeping unchanged panels' queries and mutators stable needed a ref synced in an insertion effect. The follow-up does it with updater-based setters instead.
…035-reduce-lag-when-expanding-or-collapsing-metrics # Conflicts: # static/gsApp/overrides/useButtonTracking.spec.tsx
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem: Expanding or collapsing a metric panel writes to the URL. Every panel re-decoded its query params from the URL, and table rows and cells subscribed to the location for their links. So any expansion/collapse -> causes a URL change -> causes every component in every other panel to re-render.
This PR fixes it with a few things:
As part of this I extracted
renderTraceCellout into aTraceLinkCellcomponent.Keeping unchanged panels' query params and mutators stable is split out into #128357.
Together with getsentry/sentry-javascript#25336 I think this should fix much of the perf issues around metrics panel toggles.
These numbers were measured before #128357 was split out of this PR.
Closes LOGS-1035.