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

RTK 2.0 Migration: upsertQueryEntries does not allow refetch behavior #4717

Open
JacobJaffe opened this issue Nov 18, 2024 · 11 comments
Open

Comments

@JacobJaffe
Copy link

JacobJaffe commented Nov 18, 2024

Hi, I've been testing out migrating from upsertQueryData to upsertQueryEntries.

Before:

          await Promise.all(
            posts.map((post) =>
              dispatch(
                postsApi.util.upsertQueryData(
                  "getPost",
                  {
                    id: post._id,
                  },
                  post
                )
              )
            )
          );

After:

          dispatch(
            postsApi.util.upsertQueryEntries(
              posts.map((post) => ({
                endpointName: "getPost",
                arg: {
                  id: post._id,
                },
                value: post,
              }))
            )
          );

However, I've found that the refetching no longer works when initialized this way. I've verified that the tags get invalidated, but no new query gets fired off.

const { refetch } = useGetPost({ id: "foo" });
...

// does not work when the query is hydrated by `upsertQueryEntries`
refetch()

This is unexpected because this is a different behavior than the mapped-upsertQueryData behavior, and feels like a bug- I've connected a component via a useQuery hook, so the subscription should be valid.

@JacobJaffe JacobJaffe changed the title upsertQueryEntries does not allow refetch behavior RTK 2.0 Migration: upsertQueryEntries does not allow refetch behavior Nov 18, 2024
@markerikson
Copy link
Collaborator

Yep, there was a similar report over in #4393 . Haven't had time to look into it yet.

@JacobJaffe
Copy link
Author

I think you're referring to this comment?

FWIW, I'm not seeing any regressions when rolling back to upsertQueryData (as mentioned in that comment), just this issue with upsertQueryEntries.

@markerikson
Copy link
Collaborator

Trying to look into this now, and I'm actually kind of confused.

I threw together a test locally that appears to pass:

 test.only('Allows refetching an upserted entry', async () => {
    const api = createApi({
      baseQuery: fakeBaseQuery(),
      tagTypes: ['Post'],
      endpoints: (build) => ({
        postFetchCounter: build.query<Post, string>({
          queryFn: async (id) => {
            if (!postCounters[id]) {
              postCounters[id] = 0
            }

            postCounters[id]++

            const post = posts.find((p) => p.id === id)!

            console.log('fetching', id, postCounters[id])

            return {
              data: {
                ...post,
                title: `Post ${id} fetch ${postCounters[id]}`,
              },
            }
          },
        }),
      }),
    })

    const storeRef = setupApiStore(
      api,
      {
        ...actionsReducer,
      },
      { withoutListeners: true },
    )

    const entriesAction = api.util.upsertQueryEntries([
      ...posts.map((post) => ({
        endpointName: 'postFetchCounter' as const,
        arg: post.id,
        value: post,
      })),
    ])

    storeRef.store.dispatch(entriesAction)
    await delay(1)

    function PostComponent({ id }: { id: string }) {
      const queryRes = api.endpoints.postFetchCounter.useQuery(id)
      const { data, refetch } = queryRes

      console.log('Query res: ', queryRes)
      return (
        <div>
          <div data-testid="title">{data?.title}</div>
          <div data-testid="status">{queryRes.status}</div>
          <button onClick={refetch}>Refetch</button>
        </div>
      )
    }

    render(<PostComponent id={posts[0].id} />, { wrapper: storeRef.wrapper })
    // Let's make sure we actually fetch, and we increment
    expect(screen.getByTestId('title').textContent).toBe(posts[0].title)

    fireEvent.click(screen.getByText('Refetch'))

    await waitFor(() => {
      expect(screen.getByTestId('status').textContent).toBe('fulfilled')
    })
    expect(screen.getByTestId('title').textContent).toBe(
      `Post ${posts[0].id} fetch 1`,
    )
  })

More specifically, I can see the queryFn for that endpoint executing, which means that the refetch does appear to be working, and that the result comes back updated.

@JacobJaffe any chance you can put together a repro of this for me to look at?

@JacobJaffe
Copy link
Author

JacobJaffe commented Nov 24, 2024

Very strange -- Also seeing it working in simple cases. I am consistently seeing it in my project however, but its a large codebase and difficult to isolate what the difference is.

I'll work on paring down the project to an MVP of the issue, and update this thread when I have that.

@markerikson
Copy link
Collaborator

@JacobJaffe Thanks! Alternately, if you can make a Replay of that happening and share it (if needed, share it privately to just [email protected] ), I could take a look at it.

@MateuszGroth
Copy link

I have a similar issue, where the usage of upsertQueryEntries breaks query invalidation

@MateuszGroth
Copy link

MateuszGroth commented Dec 4, 2024

I believe it is a combination of `invalidationBehavior: "delayed", and the upsertQueryEntries

@MateuszGroth
Copy link

MateuszGroth commented Dec 4, 2024

I have the following setup:

// 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 prefetched

Actual Result:
the data is not refetched

@MateuszGroth
Copy link

I will create a new issue tho

@JacobJaffe
Copy link
Author

JacobJaffe commented Dec 4, 2024

I believe it is a combination of `invalidationBehavior: "delayed", and the upsertQueryEntries

FWIW, my scenario is ocuring with invalidationBehavior: "immediately"

Hoping get a minimal repro of this soon. Unfortunately my project is a React Native codebase, so wiring up a Replay isn't in the cards.

@markerikson
Copy link
Collaborator

It looks like this is likely the same issue as #4749 , where some broken internal logic leaves the updated query entry as "pending". @JacobJaffe , can you confirm if that's what's happening in your case?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants