genetic-orange

`enabled: false` returning cached data

Hi there! We've noticed a bit of a footgun in our codebase. Folks on our team will use useQuery and pass in a query object that depends on some data. If that data isn't available or isn't loaded yet, they'll set enabled: false to disable the query. However, disabling a query only prevents the query from running, and it may still return cached data (see https://tanstack.com/query/latest/docs/framework/react/guides/disabling-queries). People then write the rest of the code expecting data to be undefined, but in prod that data will sometimes happen to be cached by some other part of the app and users will get all of that cached data instead of undefined and the UI will break or behave unexpectedly. Our solution was to overwrite useQuery to have the resulting data always be undefined if enabled is false since that's what people on our team expect to happen. Is there any downside to doing this? I've attached our overwritten useQuery with what I believe are the correct types (I copy-pasted most of the code from the useQuery implementation). I don't love having to use useMemo but it seemed preferable to updating data directly on the returned object.

useQuery.ts

13 Replies

other-emerald•3w ago

there's a subscribed option, which prevents the query from getting subscribed to the cache data useful when using react-native too, as screens are kept rendered in the background, so you'd usually do subscribed: isFocused to disable them when their screen is not focused

sensitive-blue•3w ago

If that data isn't available or isn't loaded yet, they'll set enabled: false to disable the query.

There's something smelly here. My guess is you aren't putting all the dependencies of the query into the queryKey, so you're seeing stale data from some other dependencies? if you have:

useQuery({
  queryKey: ['something', myVariable],
  queryFn: () => fetchSomething(myVariable),
  enabled: !!myVariable
}

useQuery({
  queryKey: ['something', myVariable],
  queryFn: () => fetchSomething(myVariable),
  enabled: !!myVariable
}

there's no way you get some data if myVariable is "not ready yet", because the queryFn will never have run for that key.

sensitive-blue•3w ago

also: - your safeResult useMemo doesn't do anything because queryResult will be a new object every render. - by calling ...queryResult you opt out of some optimization that we have (tracked properties) We have lint rules for all of these gotchas, I recommend using our eslint plugin to catch them: https://tanstack.com/query/latest/docs/eslint/eslint-plugin-query

ESLint Plugin Query | TanStack Query Docs

TanStack Query comes with its own ESLint plugin. This plugin is used to enforce best practices and to help you avoid common mistakes. Installation The plugin is a separate package that you need to ins...

genetic-orangeOP•3w ago

Thank you so much for your responses! FWIW we do have the eslint plugin installed, but maybe something is up with our implementation that it's not catching those gotchas. And even if we have all the dependencies set properly in queryKey, we can still have a query somewhere else in the app that's unexpectedly populating a query's cache. As a toy example:

const queryURL = allActiveAppointments()
const result = useQuery({ queryKey: ['Appointment', queryURL] })

const queryURL = allActiveAppointments()
const result = useQuery({ queryKey: ['Appointment', queryURL] })

function allActiveAppointments() {
  return `appointments/${normalize({active: true})}`
}

function allActiveAppointments() {
  return `appointments/${normalize({active: true})}`
}

const relatedIds = [] // Empty while loading or if there just happen to be none.
const queryURL = appointmentsByIds(relatedIds)
const result = useQuery({ queryKey: ['Appointment', queryURL], enabled: relatedIds.length > 0 })
// This will return the result from the allActiveAppointments query above somewhere else in the app even if enabled is set to false.

const relatedIds = [] // Empty while loading or if there just happen to be none.
const queryURL = appointmentsByIds(relatedIds)
const result = useQuery({ queryKey: ['Appointment', queryURL], enabled: relatedIds.length > 0 })
// This will return the result from the allActiveAppointments query above somewhere else in the app even if enabled is set to false.

function appointmentsByIds(ids) {
  return `appointments/${normalize({active: true, ids})}` // Note the normalize function clears empty arrays from the query string.
}

function appointmentsByIds(ids) {
  return `appointments/${normalize({active: true, ids})}` // Note the normalize function clears empty arrays from the query string.
}

genetic-orangeOP•3w ago

And say we did go this route, in this particular case, would it be preferable to update data directly on the returned object and do queryResult.data = undefined? I was wary of doing that since I don't know if data is used internally somehow and directly changing the object would have unintended consequences.

sensitive-blue•3w ago

Why would a query be disabled if you have no relatedIds and then not have relatedIds part of the queryFn / the request you make ?

genetic-orangeOP•2w ago

Here is an example. We're querying by a list of ids (conditionRefs). If the list of ids is empty, we want to show nothing instead of showing everything. The query ends up being for every Diagnosis because empty arrays get normalized away and the query ends up being much broader than intended. If React hooks didn't require stable positioning across renders, we would omit the query hook entirely when the list of ids is empty. An alternative pattern we use is:

useSearch(Diagnosis, conditionRefs.length > 0 ? { _id: conditionRefs } : undefined)

useSearch(Diagnosis, conditionRefs.length > 0 ? { _id: conditionRefs } : undefined)

But not everyone on our team uses that pattern and instead try to use enabled which leads to the caching issue.

other-emerald•2w ago

But if _id makes it into the queryKey that shouldn't be an issue? Unless I'm missing something. If your queryKey is unique for the "no ids" case there would be no way it could reuse cached data from a previous query (You should also really put that sortBy() inside the useQuery's select)

genetic-orangeOP•2w ago

👀 I haven't used select before – thank you!

genetic-orangeOP•2w ago

_id doesn't make it into the queryKey – since it's an empty array, it gets normalized away by normalizeQuery. So the queryKey for "no ids" ends up being the same as the queryKey for "all data." One solution is to make sure everyone uses the alternative pattern above and makes it explicit that it should be "no ids" and not "all data": useSearch(Diagnosis, conditionRefs.length > 0 ? { _id: conditionRefs } : undefined) Unfortunately it's easy to forget to do this with dynamic array parameters. Note we disable exhaustive-deps in the screenshot because resourceClass is a class. fhirSearchUrl is a stringified version of the parameters and resource type so in theory it should be safe to do.

other-emerald•2w ago

Cache works based off the queryKey so the only advisable approach here is to differentiate it. I'm not entirely sure how to achieve it with your current approach The easiest would probably be to put the query in a child component and only render it when you do in fact have conditionRefs.length > 0, then you bypass the problem altogether and don't run any query code when it's not needed. The underlying problem is indeed that you ideally want a conditional hook but react doesn't allow that - the next best thing is a conditional component that executes the hook. Alternatively, you'd probably want to build in support for an enabled flag in your useSearch and searchOptions and have it affect the internal queryKey. This isn't exactly a great solution because affecting the queryKey would only be necessary in those cases where your queryKey should be made up of dynamic parameters that you're currently lacking. There's also a simpler use-case where you want to disable the query but have all the parameters so don't need queryKey manipulation. Sorry, getting a bit complicated and not explaining it very well 🙂 I'd go with the conditional component in this case to be honest. Dealing with missing parameters in a query hook is otherwise always going to be annoying and lead to weird stuff.

sensitive-blue•2w ago

So the queryKey for "no ids" ends up being the same as the queryKey for "all data."

that's probably the real issue here

genetic-orangeOP•2w ago

Thank you so much to all of you! This has been really helpful 🙏

Gaming

Programming

`enabled: false` returning cached data

Did you find this page helpful?