ref(charts): replace useRouter with specific hooks in useChartZoom#110094
ref(charts): replace useRouter with specific hooks in useChartZoom#110094evanpurkhiser wants to merge 15 commits into
Conversation
Moves the "Test Monitor" button to the Verification section header as a trailing item, and moves the "Suggest Assertions" button next to the "+ Add Assertion" button in the assertions field. Fixes [NEW-763](https://linear.app/getsentry/issue/NEW-763)
…riptions Fixes NEW-761
github-actions
Bot
added
Scope: Frontend
|
🚨 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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| end: endFormatted, | ||
| }, | ||
| router, | ||
| null, |
There was a problem hiding this comment.
Passing null router breaks URL sync on chart zoom
High Severity
Passing null as the second argument to updateDateTime causes updateParams to early-return without updating URL query parameters (it checks if (!router) { return; }). Previously the legacy router object was passed, which allowed updateDateTime to sync the zoomed date range into the URL. Now the PageFiltersStore is updated but the URL never reflects the zoom, breaking URL-based state for any consumer using saveOnZoom (e.g. automationStatsChart.tsx). This means zoomed state is lost on page refresh and can't be shared via URL.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| null, | ||
| {save: saveOnZoom} | ||
| ); | ||
| } | ||
| }, |
There was a problem hiding this comment.
Bug: Passing a null router to updateDateTime prevents the chart's zoom state from being saved to the URL, breaking state persistence on refresh or navigation.
Severity: HIGH
Suggested Fix
Instead of passing null to updateDateTime, use the useNavigate hook to manually update the URL's query parameters. Construct a new query object containing the updated time range and use navigate to apply it. This will ensure the URL state remains synchronized with the page filter store, restoring persistence.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: static/app/components/charts/useChartZoom.tsx#L182-L186
Potential issue: In `useChartZoom.tsx`, the call to `updateDateTime` was changed to pass
`null` for the `router` argument. The `updateDateTime` function relies on this `router`
object to update the URL's query parameters via `updateParams`. When `router` is `null`,
`updateParams` returns early, and the URL is never updated. This causes a state mismatch
where the chart's zoom level is updated in the page filters store but not reflected in
the URL. Consequently, sharing links, using browser history, or refreshing the page will
result in the loss of the selected zoom range for components that use `saveOnZoom:
true`, such as issue detail charts and dashboard widgets.
Did we get this right? 👍 / 👎 to inform future reviews.
|
This PR has a migration; here is the generated SQL for for --
-- Raw Python operation
--
-- THIS OPERATION CANNOT BE WRITTEN AS SQL |
1 participant
Part of the React Router 6 cleanup — replaces
useRouter()with specific hooks (useNavigate,useLocation,useParams, etc.)