redux-toolkit icon indicating copy to clipboard operation
redux-toolkit copied to clipboard

Document how to batch RTK Query mutations properly

Open torhovland opened this issue 2 years ago • 5 comments

I feel that the documentation needs to cover the following scenario.

I have an array of objects that I need to send to the API. The API only allows mutating one object per request. What is a good way to do this while:

  • avoiding a race condition where the final cache invalidation is ignored because the second to last cache provide comes later.
  • even better, firing just one cache invalidation after all mutations have been sent.

I understand mutations can probably use a queryFn to do this, but the docs only cover how to do multiple fetches.

torhovland avatar Apr 05 '22 08:04 torhovland

From the "endpoint specification" side, there is literally no difference between queries and mutations, apart that queries provideTags and mutations invalidateTags. Doing it with a queryFn is a fine way to do it - with exactly the code as you would do with a query.

phryneas avatar Apr 05 '22 08:04 phryneas

Thanks, that helped. I was able to get it working using this:

    updateAccessLevelRequests: builder.mutation<number, AccessLevelRequest[]>({
      async queryFn(accessLevelRequests, _queryApi, _extraOptions, baseQuery) {
        for (const accessLevelRequest of accessLevelRequests) {
          await baseQuery({
            url: `access-level-requests/${accessLevelRequest.id}/`,
            method: 'PUT',
            body: accessLevelRequest,
          });
        }
        return { data: accessLevelRequests.length };
      },
      invalidatesTags: ['AccessRequests'],
    }),

I still think it would be good to show this in the docs, though.

torhovland avatar Apr 05 '22 09:04 torhovland

PRs to the docs are always welcome :)

phryneas avatar Apr 05 '22 09:04 phryneas

I know. For now I wanted to at least record it as an issue.

torhovland avatar Apr 05 '22 09:04 torhovland

@phryneas just wanted to add that this is not a good solution when using an auto generated RTK api file. manually adding queries in this use case is beating the purpose. RTK Query needs a proper way to batch multiple mutations and merge their respective invalidation tags when they're both done. otherwise there are use cases where race conditions occur.

giladv avatar Jul 03 '22 12:07 giladv

Agree with @giladv

I have to call two mutations on an update of a record. Two different endpoints. Both invalidate the same tag. Frequently results in race conditions and the user only sees part of the update in the UI.

And the API file is generated from OpenAPI

slashwhatever avatar Dec 09 '22 16:12 slashwhatever

Short answer is that isn't a use case we considered in the initial design, there haven't been many requests for it (other than this thread), and designing that functionality isn't a thing that's on our radar.

Not saying it's impossible or will never be done, just that it's not a thing we've spent time thinking about.

markerikson avatar Dec 09 '22 16:12 markerikson

Removing the actual implementation details for a moment, is it not a concern that two mutations called in quick succession - irrespective of what they are doing - would cause a race condition if they try an invalidate the same tag?

To me that sounds like a bug that could raise it's head in any number of scenarios outside of the one in this thread.

slashwhatever avatar Dec 12 '22 15:12 slashwhatever

@slashwhatever : I don't see it as a "bug" in the sense of "the library code is behaving in an unexpected or unintended way", no. I would potentially see it as a limitation in the design, but the library code is working as intended as far as I know.

markerikson avatar Dec 12 '22 16:12 markerikson

Are more use-cases helpful for you? If so:

Currently, I have a list of document ids of unknown length and I need to fetch metadata and preview content for all those documents. But every document needs its own request. And I need this metadata for further calculations, not in separate components where this wouldn't be a problem.

E.g.:

const documentIds = ['1', '2', '3', '4']

Would result in those requests:

GET /api/document/1/metadata
GET /api/document/1/preview
GET /api/document/2/metadata
GET /api/document/2/preview
etc.

alinnert avatar Dec 20 '22 12:12 alinnert

Here's an example of how I resolved my use case:

    updateAllTheThings: build.mutation<
      UpdateThingsResponse | FetchBaseQueryError,
      UpdateThingsRequest & SetOtherThingsRequest
    >({
      async queryFn(queryArg, _queryApi, _extraOptions, fetchWithBaseQuery) {
        const { thingId, foo, bar } = queryArg.body;
        const { baz } = queryArg.body;

        const updateOneThing = await fetchWithBaseQuery({
          url: `thingEndpoint/thing/${queryArg.thingId}/set-things`,
          method: "PATCH",
          body: { baz },
        });

        if (updateOneThing.error) return { error: updateOneThing.error };

        const updateOtherThing = await fetchWithBaseQuery({
          url: `otherThingEndpoint/otherThing/${queryArg.thingId}`,
          method: "PATCH",
          body: { foo, bar },
        });

        if (updateOtherThing.error) return { error: updateOtherThing.error };

        return { data: updateOtherThing.data as UpdateThingsResponse };
      },
      invalidatesTags: ["Things"],
    }),

In this specific case, the order of requests was important. If the first failed, it should not send the second and return the error. There wasn't any way around the problem that we generate our code from OpenAPI but we've dealt with that internally. You could, I guess, set up another extended api for your manually created ones and just inject it with api.injectEndpoints

@alinnert Not sure if this helps you think about how to solve for your use case, but I figured it was worth sharing in case someone gets some use from it.

slashwhatever avatar Dec 20 '22 12:12 slashwhatever

Here's another use case with a workaround.

Use case:

const { data: items, isFetching: itemsIsFetching, isSuccess: itemsIsSuccess } = useGetItemQuery();
const [ updateItemName ] = usePutItemNameMutation();
const [ updateItemDescription ] = usePutItemDescriptionMutation();

React.useEffect(() => {
    // Run only when fetching is complete
    if (itemsIsFetching || !itemsIsSuccess) return;
    // Perform actions based on the updated item list
    // ...
}, [itemsIsFetching]);

const onSave = async () => {
    await updateItemName("itemId", "someName");
    await updateItemDescription("itemId", "someDescription");
};

The item endpoint represents an object in the database. Each of its properties can be updated with a separate PUT method, which makes it easier to implement granular permissions - some users can update the description and other nested properties, while others cannot. Sometimes, we want to update two separate properties at once. This, however, results in a race condition where the re-fetch action is fired immediately after the first update. The second update is performed before the GET request finishes and doesn't trigger another refetch. This causes the useEffect hook to be fired after the first update, but before the second one, so it cannot access the updated items[number].description value. In this particular case, we don't have to send the updates sequentially, as they don't depend on each other, but that's beside the point. We are using @rtk-query/codegen-openapi to generate and inject the endpoints, which makes it impossible to use the queryFn workaround.

Workaround:

const [skip, setSkip] = React.useState(false);
const { data: items, isFetching: itemsIsFetching, isSuccess: itemsIsSuccess } = useGetItemQuery({ skip });
//...
const onSave = async () => {
    setSkip(true); // The items list is emptied
    await updateItemName("someName");
    await updateItemDescription("someDescription");
    setSkip(false); // Triggers a reload
};

There are many drawbacks to this workaround, but it works - it disables refetching of the item list while the updates are being applied, and then enables it, which causes the useEffect hook to fire after the second update.

It would be best if we could batch multiple mutations, as proposed by giladv.

TKasperczyk avatar Jan 10 '23 18:01 TKasperczyk

I'm also having this issue. Consider the following example:

list: builder.query({
   providesTags: () => ["Item"]
}),
update: builder.mutation({
   invalidatesTags: () => ["Item"]
})

const [list] = useListQuery()
const [update] = useUpdateMutation()

const onChange = async () => {
    const result1 = await update(...) // Update item 1
    if ("error" in result1) {
         showErrorToast()
    } else {
        const result2 = await update(...) // Update item 2
    }
}

After onChange has run I'm left with a list of items that doesnt have the updates to item 2. It seems as you can't invalidte a query while it's running? Tried to add a manual refetch after update2 has finished but if that happens while the list-query is still fetching the invalidation seems to be invalidated?

marcus13371337 avatar Jan 30 '23 10:01 marcus13371337

@marcus13371337 You could give the CI build of https://github.com/reduxjs/redux-toolkit/pull/3116 a try and see if that is what you need.

phryneas avatar Jan 30 '23 13:01 phryneas

A hopeful fix for this is now available in https://github.com/reduxjs/redux-toolkit/releases/tag/v2.0.0-beta.4 ! Please try it out and let us know if it resolves the concerns.

markerikson avatar Oct 28 '23 23:10 markerikson

Haven't heard anything. Will assume this is fixed unless someone says otherwise!

markerikson avatar Nov 14 '23 20:11 markerikson