Bug: cache invalidation of queries that use serializeQueryArgs

[SebastienGllmt]

If you specify serializeQueryArgs it seems the queries don't get invalidated properly when you call invalidateTags.

Notably, the endpoint does get invalidated and rerun as expected, but the arguments passed in are the old arguments when the cache key was created and not what they are after the invalidation

I added a test that shows this bug in this PR, but I don't have a fix for it.

Here is pseudo code if you prefer that:

getFoo: {
    query: request => {
       console.log(request); // this will always print the `request` value when initially cached
    },
    serializeQueryArgs: ({ queryArgs, endpointDefinition, endpointName }) => {
        return defaultSerializeQueryArgs({
          endpointName,
          queryArgs: {
            foo: queryArgs.foo,
          },
          endpointDefinition,
        });
    },
    providesTags: ["FooTag"]
}

query({ foo: 5, bar: 10})
invalidateTags(["FooTag"])
query({ foo: 5, bar: 15}) // this triggers the rtk query, but "request" still has bar=10

[codesandbox]

  Open in CodeSandbox Web Editor | VS Code | VS Code Insiders

[netlify]

✅ Deploy Preview for redux-starter-kit-docs ready!

Name	Link
🔨 Latest commit	00f9261
🔍 Latest deploy log	https://app.netlify.com/sites/redux-starter-kit-docs/deploys/63dad1b7c86a330008b3f189
😎 Deploy Preview	https://deploy-preview-3147--redux-starter-kit-docs.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site settings.

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 00f9261:

Sandbox	Source
Vanilla	Configuration
Vanilla Typescript	Configuration
rsk-github-issues-example	Configuration
@examples-query-react/basic	Configuration
@examples-query-react/advanced	Configuration
@examples-action-listener/counter	Configuration

[SebastienGllmt]

Debugging this a bit more, it seems like the following is happening:

1. First query runs with bar: 10 and the result is cached
2. invalidateTags invalidates the query, but the invalidation causes the query to rerun right away using bar: 10 again and that gets cached
3. When calling with bar: 15, nothing is run because it simply pulls from the cache created in (2)

[SebastienGllmt]

I tried looking into adding something like

if ('serializeQueryArgs' in endpointDefinitions[endpointName]) {
  mwApi.dispatch(
    removeQueryResult({
      queryCacheKey: queryCacheKey as QueryCacheKey,
    })
  )
}

inside the invalidateTags function, but I fear there is no way to get this to work properly given other parts of the library expect the refetchQuery to execute as expected inside invalidateTags if there is a subscriber.

Maybe you can think of a way to solve this, but I also feel like maybe my use-case of having arguments that are only important enough to trigger a re-query after invalidation may be an abuse of API. Instead, it may make more sense to mention this behavior in the docs and recommend people to instead use WeakMap-based caching if you want to get a unique ID to put inside your serializeQueryArgs that either can't be serialized (or is too big to be serializable)

[SebastienGllmt]

This is a copy-paste mistake and should have been 'd', 'e', 'f'

[phryneas]

I'm just skimming over this, but couldn't this be solved here?

redux-toolkit/packages/toolkit/src/query/core/buildSlice.ts

Lines 161 to 163 in 0ce2ec0

	if (arg.originalArgs !== undefined) {
	substate.originalArgs = arg.originalArgs
	}

[SebastienGllmt]

recommend people to instead use WeakMap-based caching if you want to get a unique ID to put inside your serializeQueryArgs

I tried this and unfortunately this isn't an option either. This seems to be because if you have JSON.stringify(queryArgs1) === JSON.stringify(queryArgs2) but cache(queryArgs1) !== cache(queryArgs2) it will cause your query to be stuck forever.

AKA
✔️many serialize(queryArgs) → 1 cache
❌1 serialize(queryArgs) → many chache

Haven't found the cause for this though, so I could be wrong on my guess

[SebastienGllmt]

Alright, yet another attempt at this: use forceRefetch on the endpoint to force a refetch when bar: 15 even if the cache key is different

forceRefetch: ({ currentArg, previousArg }) => { 
    return currentArg !== previousArg
}

This works in practice, but doesn't work in the test case because forceRefetch isn't even called during the query({ foo: 5, bar: 15}) query. It looks like this is because of the following line in buildThunks

// Don't retry a request that's currently in-flight
if (requestState?.status === 'pending') {
    return false
}

[markerikson]

@SebastienGllmt : thanks for the PR! I'll try to look at it "soon" (although I've got other things going on the next few days).

That said, can you clarify what actual use cases you're trying to solve atm that led to this?

[SebastienGllmt]

We have an RTK Query where the input to the query is large enough that the serialization logic to create the cache key affects performance, so we want to remove it from the cache. The combination of serializeQueryArgs and forceRefetch seems to have done the trick for our application because now we can keep the cache key short, yet still refetch the data when necssary

When I created this PR initially, I didn't realize I could use forceRefetch for this, so I was trying to keep the cache key short and manually trigger refetching by invalidating tags

We're still getting some performance hit because it looks like RTK Query does some JSON stringification of the arguments internally even if it's not part of the cache key, but this is manageable (about 50ms of our app startup is spent on this stringification of arguments)

[markerikson]

Gotcha.

Hmm. I wonder if we could do some basic WeakMap-based caching of args for defaultSerializeQueryArgs?

[markerikson]

@SebastienGllmt : could you do me a favor and either generate a JS devtools CPU perf of your app (can send it to me privately on Discord if you want), or put together a quick example that shows the stringification taking up time?

Also, could you try out the CodeSandbox CI build from #3193 and see if that helps improve perf for your app?