invalidateQueries with refetchActive: false on inactive query refetches it

[donysukardi]

Describe the bug
Calling queryClient.invalidateQueries with { refetchActive: false } on inactive query actually refetches it.

To Reproduce
Codesandbox: https://codesandbox.io/s/react-query-refetchactive-bug-n3kwc

With the following query cache state:

['queryA'] // inactive
['queryB'] // active

When I call

// ❌ Incorrect, refetches 😵
queryClient.invalidateQueries(['queryA'], {
  refetchActive: false
});

// ✅ Correct, doesn't refetch
queryClient.invalidateQueries(['queryB'], {
  refetchActive: false
});

Expected behavior
Option { refetchActive: false } on inactive query shouldn't trigger refetch

Screenshots
If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

• OS: macOS
• Browser: chrome
• Version: 90

Additional context
N.A

[TkDodo]

I don't understand the codesandbox example. Do I need to change something in the code? How do I trigger the invalidation? I just see a B on the screen 😅 🙈

[donysukardi]

Oh.. I forgot to add instruction to look at the console to see the sequence of the fetches 😬

The example mounts and unmounts queryA, then mounts queryB and finally invalidates both queries.

function Example() {
  const queryClient = useQueryClient();
  const [shouldRenderB, setShouldRenderB] = React.useState(false);

  React.useEffect(() => {
    if (shouldRenderB) {
      setTimeout(() => {
        queryClient.invalidateQueries(["queryA"], { refetchActive: false });
        queryClient.invalidateQueries(["queryB"], { refetchActive: false });
      }, 500);
    }
  }, [shouldRenderB, queryClient]);

  return (
    <div>
      <p>Open console to see the fetching sequence</p>
      {!shouldRenderB && <CompA onSuccess={() => setShouldRenderB(true)} />}
      {shouldRenderB && <CompB />}
      <ReactQueryDevtools initialIsOpen={false} />
    </div>
  );
}

[TkDodo]

it looks like there is a bug in matchQuery: the query is "found" even though it shouldn't be. Basically:

• invalidateQueries sets QueryFilters to active here: https://github.com/tannerlinsley/react-query/blob/09116e54b98f5f15fd6777fb26a3d735573a2e4a/src/core/queryClient.ts#L241 and forwards it to refetchQueries
• there, we findAll queries and refetch the matching ones: https://github.com/tannerlinsley/react-query/blob/09116e54b98f5f15fd6777fb26a3d735573a2e4a/src/core/queryClient.ts#L270
• I quickly debugged and queryA is correctly seen as inactive in the cache, but for some reason, it still matches must be somewhere here: https://github.com/tannerlinsley/react-query/blob/4083c1ef8a8734d2680a4e5f031c7958c18ee176/src/core/utils.ts#L167-L216

Do you want to take a stab at it? queryCache.findAll has lots of tests, but maybe we are missing this case and you can add a test and see where it takes you ...

[donysukardi]

I figured as much. I was debugging all the way till matchQuery 🙈.

Sure, I can take a stab at it.

What would be the intended behaviour if both active and inactive are set? It's currently possible to provide both. Given the following query cache state,

queryA: inactive
queryB: active

this would be the expected matrix?

refetchActive	refetchInactive	results
true	true	[queryA, queryB]
true	false	[queryB]
false	true	[queryA]
false	false	[]

[TkDodo]

Sure, I can take a stab at it.

awesome, thanks!

this would be the expected matrix?

I would say so, yes :)

[donysukardi]

Can get your understanding of the following for matchQuery, for single active/inactive state,

• is { active: false } the same as { inactive: true }?
• is { inactive: false } the same as { active: true }?

active	inactive	results
true		active queries
	true	inactive queries
false		???
	false	???

It becomes complicated and ambiguous if we allow both active and inactive to be set in the same config.

I'm wondering if it's better if we split the refetch in invalidateQueries itself.

invalidateQueries(
  filters?: InvalidateQueryFilters,
  options?: InvalidateOptions
): Promise<void>
invalidateQueries(
  queryKey?: QueryKey,
  filters?: InvalidateQueryFilters,
  options?: InvalidateOptions
): Promise<void>
invalidateQueries(
  arg1?: QueryKey | InvalidateQueryFilters,
  arg2?: InvalidateQueryFilters | InvalidateOptions,
  arg3?: InvalidateOptions
): Promise<void> {
  const [filters, options] = parseFilterArgs(arg1, arg2, arg3)

  const refetchActive = filters.refetchActive ?? true
  const refetchInactive = filters.refetchInactive ?? false

  return notifyManager.batch(() => {
    this.queryCache.findAll(filters).forEach(query => {
      query.invalidate()
    })
    const promises = []
    if (refetchActive) {
      promises.push(
        this.refetchQueries({ ...filters, active: true }, options)
      )
    }
    if (refetchInactive) {
      promises.push(
        this.refetchQueries({ ...filters, inactive: true }, options)
      )
    }
    
    let promise = Promise.all(promises).then(noop)
    if (!options?.throwOnError) {
      promise = promise.catch(noop)
    }

    return promise
  })
}

[TkDodo]

I agree having two boolean flags is sub-optimal. ideally, it would be a tri-state enum like 'all' | 'active' | 'inactive', but we can't really change the api now :/

the split I don't quite understand, because refetchQueries is a public api in itself, so we could also do:

queryClient.refetchQueries({ active: true, inactive: true })

so this a case that we need to support anyhow

this is the normal QueryFilters filter object that applies to many public queryClient methods, like refetchQueries, removeQueries, resetQueries etc. They all just pass the filters down to queryCache.findAll, which then calls matchQuery. The only difference with invalidateQueries is that it, per default, only wants to refetch active filters, which is why it falls back to true on the refetchActive input.

So I think having both to true is the same as having both to undefined. Maybe the logic in matchQuery can be simplified by using default values when destructing the filters, because we wouldn't have to deal with an input being undefined anymore 🤔 . This is quite hard to grasp imo:

https://github.com/tannerlinsley/react-query/blob/4083c1ef8a8734d2680a4e5f031c7958c18ee176/src/core/utils.ts#L191-L201

[donysukardi]

We need to put in none in the mix as well.

active	inactive	results
true	true	all
true	false	active
false	true	inactive
false	false	none
undefined	undefined	all

But if we default destructuring to false, then we will get no results when both are not present. We should still check if user has explicitly passed nothing for active and inactive, which in that case, it should be treated as all.

[TkDodo]

But if we default destructuring to false, then we will get no results when both are not present.

I would've destructed to true, but I'm not sure this is right either. The docs say:

active?: boolean
  When set to true it will match active queries.
  When set to false it will match inactive queries.
inactive?: boolean
  When set to true it will match inactive queries.
  When set to false it will match active queries.

this means that inactive: false is the same as active:true ? I'm so confused right now 😅

[tannerlinsley]

🎉 This issue has been resolved in version 3.18.1 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀