ref(dashboards): migrate issues dataset to hook pattern#106780
Conversation
github-actions
bot
added
the
Scope: Frontend
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Use stable refs for fetchFnRef objects to enable proper queue deduplication. Previously, new objects were created on each query execution, breaking the queue's reference equality check and causing duplicate API calls. - Create fetchFnRefsRef array using useRef to store stable refs - Initialize refs in useMemo when queryKeys change - Pass queryIndex to createQueryFn to use the correct stable ref - Apply fix to both useIssuesSeriesQuery and useIssuesTableQuery Fixes the issue identified by Cursor Bugbot where queue deduplication was broken due to unstable ref objects.
Fix ESLint prefer-promise-reject-errors violations by wrapping non-Error rejection values in Error objects.
…eryExtras - Use `data` instead of `query` for table query params to match existing API patterns - Flatten `queryExtras` into query params for series queries so `category` is at top level
|
@cursoragent review |
|
Unable to authenticate your request. Please make sure to connect your GitHub account to Cursor. Go to Cursor |
|
@cursor review |
Removes GroupStore.add() side effect from transformIssuesResponseToTable (making it a pure function) and moves it to a useEffect in useIssuesTableQuery hook to avoid React warning about updating a component while rendering a different component.
narsaynorath
left a comment
narsaynorath
left a comment
There was a problem hiding this comment.
Just a question about something new I saw
| const noop = () => Promise.resolve(); | ||
| while (fetchFnRefsRef.current.length < keys.length) { | ||
| fetchFnRefsRef.current.push({current: noop}); | ||
| } | ||
| fetchFnRefsRef.current.length = keys.length; |
There was a problem hiding this comment.
Why does this adjustment need to happen for issues? I don't believe I saw this in your other PRs so I'm just trying to understand what we're trying to balance with this
There was a problem hiding this comment.
THis was needed if the size the array changes, you might have fetchFnRefsRef.current[queryIndex] === undefined, the other datasets created the fetch functions on the fly within a useCallback.
I don't like this approach as much, so i'll update it to be consistent with the other datasets.
…es-dataset-to-hook-pattern
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.
| prevGroupDataRef.current[i] = data; | ||
| } | ||
| }); | ||
| }); |
There was a problem hiding this comment.
useEffect missing dependency array runs every render
Low Severity
The useEffect that populates GroupStore has no dependency array, causing it to run after every render. While the ref-based tracking (prevGroupDataRef) prevents duplicate GroupStore.add calls, the effect still executes unnecessarily on every re-render. Adding [queryResults] as a dependency would limit execution to only when query results actually change, following React best practices.
DominikB2014
deleted the
dominikbuszowiecki/browse-254-migrate-issues-dataset-to-hook-pattern
branch
Same as #106779 but for issues dataset
2 participants
Same as #106779 but for issues dataset