Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UpsertQueryEntries breaks query invalidation when the invalidationBehavior is set to delayed #4749

Closed
MateuszGroth opened this issue Dec 4, 2024 · 6 comments · Fixed by #4768
Labels
bug Something isn't working rtk-query

Comments

@MateuszGroth
Copy link

MateuszGroth commented Dec 4, 2024

// api configuration
invalidationBehavior: "delayed"

// queries
getQuery: build.query<>({
      query:...,
      providesTags: ["QueryTag"]
}),
editQuery: build.mutation<>({
     invalidatesTags: ["QueryTag"]
}),

getSomeUnrelatedListData: buld.query<>({
     query:...
     async onQueryStarted(_, { dispatch, queryFulfilled }) {
        try {
          const res = await queryFulfilled;
          const results = res.data;
          dispatch(
           apiI.util.upsertQueryEntries(
              results.map((result) => ({
                endpointName: "getSomeUnrelatedSingleItemData",
                arg: { id: result.id },
                value: result,
              })),
            ),
          );
        } catch (err) {
          void err;
        }
      },
    }),
}),

getSomeUnrelatedSingleItemData: buld.query<unknown, {id: number}>({
     
})

Steps:

  1. Trigger fetching the data by calling the getQuery (useGetQueryQuery())
  2. Trigger fetching the data by calling the getSomeUnrelatedListData (useGetSomeUnrelatedListDataQuery())
    • this populates entries for getSomeUnrelatedSingleItemData
  3. Trigger getQuery data invalidation by calling the editQuery mutation

Expected result:
data returned by the useGetQueryQuery() is re-fetched

Actual Result:
the data is not re-fetched

"@reduxjs/toolkit": "2.4.0",
@ceisele-r
Copy link

ceisele-r commented Dec 13, 2024

I experienced the same issue.

The issue here seems to be that the when upsertQueryEntries is called, the upserted "queries" stay in status: "pending" forever.
Image

As you can see in the Redux Browser Devtools, these are all upserted query entries that stay in status: "pending".
So it probably seems to be an issue introduced with the performance optimization in #4561 @markerikson

Since delayed is the default invalidationBehavior since v2, this potentially affects all default configurations using upserts and invalidations.

In my opinion, an upserted query entry should always directly be in status: "fulfilled" (as we just upserted the data for that query as if it was already resolved from the data source).

@markerikson
Copy link
Collaborator

markerikson commented Dec 13, 2024

Yeah, those definitely should not be in "pending" state at all. upsertQueryEntriestemporarily sets them to "pending" to mimic typical behavior, but is supposed to update them to "fulfilled" immediately after that. "pending" should never be visible externally.

Could someone provide a small repro that shows this happening? That would save me some time trying to fix this.

@markerikson markerikson added bug Something isn't working rtk-query labels Dec 13, 2024
@ceisele-r
Copy link

Hey @markerikson , thanks for the fast reply!
Here you go with a small repro: https://stackblitz.com/edit/react-ts-5ne8gqgs?file=Folder.tsx

@markerikson
Copy link
Collaborator

Okay, made a Replay that shows that repro in action:

Looks like there was a small flaw in the logic. We already had a check for the fulfilled case that bails out of writing the result if substate.requestId !== meta.requestId, so that we don't end up with conflicting request results being written. However, it also had a check for the older forced upsert logic as well, and it wasn't accounting for this newer approach.

Meanwhile, the pending logic actually keeps around the existing request ID:

      substate.requestId =
        upserting && substate.requestId
          ? // for `upsertQuery` **updates**, keep the current `requestId`
            substate.requestId
          : // for normal queries or `upsertQuery` **inserts** always update the `requestId`
            meta.requestId

So, what was happening was roughly:

  • Actual request for ID 2 resolves
  • fulfilled action for ID 2 is written, with its request ID
  • upsertQueryEntries for ID 2 is dispatched
  • Inside the reducer:
    • pending action for the upsert sees there's an existing entry, and keeps the old request ID from the actual API response, instead of overwriting it with the fake request ID from the upsert
    • Immediately after that, the fulfilled action for the upsert tries to update the entry... but the existing and incoming request IDs don't match, and it wasn't being detected as a forced upsert because that logic only was checking for the older upsertQueryData approach

and so we're left with the existing entry, updated to be in a pending state, and without the upserted data written and marked as fulfilled.

Put up #4768 with the fix . I can publish it shortly, but just to check, could someone try out the PR preview build and see if that resolves things on your end?

@ceisele-r
Copy link

Hey @markerikson thank you for the immediate fix.
I can confirm that using the build from #4768 fixes the issue in my case. The upserted "requests" now have status fulfilled and invalidation works with the default setting (invalidationBehavior: "delayed").

@MateuszGroth
Copy link
Author

Thanks for taking care of it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working rtk-query
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants