ref(ui): do not disable eslint-plugin-query for debouncing #103141
Conversation
github-actions
bot
added
the
Scope: Frontend
| const queryResult = useQuery({ | ||
| queryKey: useDebouncedValue( | ||
| useMemo( | ||
| () => ['global-dashboard-filters-tag-values', tag, selection, searchQuery], | ||
| [tag, selection, searchQuery] | ||
| ) | ||
| ), |
There was a problem hiding this comment.
this is a limitation of the eslint plugin - it doesn’t track the usage through multiple hooks. The easiest way is to just inline the hook calls, which is fine - it doesn’t violate the rules of hooks as they are all still called top-level.
we can do this if we don’t need the debounced intermediate value anywhere else.
| () => ['search-query-builder-tag-values', queryParams], | ||
| () => ['search-query-builder-tag-values', queryParams] as const, | ||
| [queryParams] | ||
| ); | ||
| const queryKey = useDebouncedValue(baseQueryKey); | ||
| const isDebouncing = baseQueryKey !== queryKey; | ||
|
|
||
| // TODO(malwilley): Display error states | ||
| const {data, isFetching} = useQuery<string[]>({ | ||
| // disable exhaustive deps because we want to debounce the query key above | ||
| // eslint-disable-next-line @tanstack/query/exhaustive-deps | ||
| const {data, isFetching} = useQuery({ | ||
| queryKey, | ||
| queryFn: () => getTagValues(...queryParams), | ||
| queryFn: ctx => getTagValues(...ctx.queryKey[1]), |
There was a problem hiding this comment.
here, we need the debounced key to calculate isDebouncing, so we can’t inline it. What we can do though is not rely on closures for the key, but use the provided queryFunctionContext: React Query gives you the QueryKey into the queryFn so that you can build self-contained functions.
To do this, we need to make sure types flow through correctly so we need:
as conston the actual key, otherwise it’s not a tuple- remove
useQuery<string[]>to justuseQuery. using<>is a type assertion in disguise and destroys type inference for the other 3 type parametersuseQueryhas, so please never do this unless absolutely necessary. Here,getTagValuesalready returns aPromise<string>, so type inference works nicely and with that, we also get type inference back for thequeryFunctionContext. That means we can now accessqueryParamsasctx.queryKey[1].
❌ 1 Tests Failed:
View the top 1 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| () => ['global-dashboard-filters-tag-values', tag, selection, searchQuery], | ||
| [tag, selection, searchQuery] | ||
| ) | ||
| ), |
There was a problem hiding this comment.
Bug: Wrong Key, Wrong Data Cache Mismatch
The queryFn uses the non-debounced searchQuery from closure while the queryKey uses the debounced value. This mismatch causes React Query to cache results under the wrong key - the query executes with the current searchQuery value but gets stored under a key containing the debounced value, potentially returning stale or incorrect cached data.
There was a problem hiding this comment.
I mean yeah, but wasn’t this the case before, too?
There are two places where the
eslint-plugin-querywas disabled because of debouncing, but there are two ways to fix this rather than working around it: inlining the queryKey or using thequeryFunctionContext.