RTK Query: Tags get merged, not replaced

[Lukas-Kullmann]

In our application, we tag all failed queries with some error tag. This works similar to the "Providing errors to the cache" documentation section.

• When the query is successful, we tag it with something that relates to the data (in the example, that's Post)
• When the query failed, we tag it with an error tag (in the example, that's UNAUTHORIZED or UNKNOWN_ERROR)

Then, we have some interaction, that invalidates all failed queries.

Expected behavior

I would expect that, if the query first failed, it has the error tag. If it then succeeds, it does NOT have the error tag, but the tag that relates to the data.
In the example, that would first be ['UNKNOWN_ERROR'] and then [{ type: 'Post', id: '1' }].

If the query first succeeds, it has the data tag. If it then fails, it does NOT have the data tag, but the error tag.
In the example, that would first be [{ type: 'Post', id: '1' }] and then ['UNKNOWN_ERROR'].

Observed behavior

What I get however is that after the tags change, the old tags are still present.
So in both cases described above, I get the tags [{ type: 'Post', id: '1' }, 'UNKNOWN_ERROR'] for my query.

This means that if I invalidate UNKOWN_ERROR later, it might refetch a query that is perfectly fine.

I find this behavior to be very confusing.
I expected that it I provide a function to providesTags, then the tags that it returns are always the ones that are valid from now on. I don't expect the previous tags to be still valid.

Code Sandbox

I created a code sandbox that highlights the problem: https://codesandbox.io/s/late-paper-2y77vs?file=/src/api.test.js

Here's an overview.

We have an API that either provides Thing or Error tag, depending on whether the query succeeded or failed.

export const thingsApi = createApi({
  reducerPath: "thingsApi",
  baseQuery: fetchBaseQuery({ baseUrl: "http://example.com/" }),
  tagTypes: ["Thing", "Error"],
  endpoints: (build) => ({
    getThings: build.query({
      query: () => "things",
      providesTags: (_data, error) => (error ? ["Error"] : ["Thing"])
    })
  })
});

In the test, we first use msw to mock the request so that it fails. We expect to have exactly one query with Error tag and zero queries with Thing tag.
This works.

Then, we change the mock so that it succeeds. Since there is no error any more, we expect zero queries with Error tag and one query with Thing tag.

  const getApisInvalidatedBy = (tag) => {
    return thingsApi.util.selectInvalidatedBy(store.getState(), [tag]);
  };

  it("should remove the Error tag when the query succeeds", async () => {
    server.use(getThingsErrorHandler);

    const startAction = store.dispatch(
      thingsApi.endpoints.getThings.initiate()
    );

    try {
      await startAction;

      expect(getApisInvalidatedBy("Thing")).toHaveLength(0);
      expect(getApisInvalidatedBy("Error")).toHaveLength(1);

      server.use(getThingsHandler);

      startAction.refetch();

      await new Promise((resolve) => {
        setTimeout(resolve, 500);
      });

      expect(getApisInvalidatedBy("Thing")).toHaveLength(1);
      expect(getApisInvalidatedBy("Error")).toHaveLength(0); // <-- it breaks here
    } finally {
      startAction.unsubscribe();
    }
  });

However, the tests fail because we have one query with Thing tag and one query with Error tag.

(Note that the "tests" tab in the code sandbox does not work unfortunately. If I run this code on my machine though, the tests fail.)

[markerikson]

Afraid I don't have time to look at this right atm, and I'm not sure what the intended behavior here is... but I'd like to say thank you for a very well-written bug report :)

[phryneas]

Very good catch!

This is definitely not intended, it should always get replaced by the last result.

Seems like this comes from this logic, which is definitely only adding tag mappings, never removing them - honestly I never anticipated this scenario, but it is a very valid use case to "change" tags completely:

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

Lines 356 to 366 in 34bfefd

	for (const { type, id } of providedTags) {
	const subscribedQueries = ((draft[type] ??= {})[
	id || '__internal_without_id'
	] ??= [])
	const alreadySubscribed =
	subscribedQueries.includes(queryCacheKey)
	if (!alreadySubscribed) {
	subscribedQueries.push(queryCacheKey)
	}
	}
	}

I'm on vacation right now and will spend the next few days traveling a lot - so it could take quite some time for me to get to this. If you have the time to look into this further: PRs are very welcome in the meantime!

[behzadmehrabi]

Can I give it a try? :)

[phryneas]

Can I give it a try? :)

Sure!