useInfiniteQuery fetchNextPage function is inconsistent with refetch

[lgenzelis]

Describe the bug

If you call fetchNextPage repeatedly, each call will result in the invocation fo the underlying async function, regardless of whether the first invocation has already resolved.
This behavior is inconsistent with that of e.g. refetch. If you call refetch 100 times in a row, before the first invocation has resolved, the underlying async function doesn't get re-invoked.

To Reproduce
Steps to reproduce the behavior:

1. Go to this sandbox
2. Open the console
3. Click on the Refetch button repeatedly. You'll notice the line Fetching... printed on the console, just once (unless you wait more than 5 seconds, which is how long the async call takes to resolve)
4. Click on the Fetch Next Page button repeatedly. You'll notice the line Fetching next page... printed on the console several times (once per button click)

Expected behavior
fetchNextPage should behave as refetch: you should see the line Fetching next page... printed only once.

Desktop (please complete the following information):

• OS: macos
• Browser: chrome
• Version: 92

[TkDodo]

it just seems that they have different default values. Both functions call this.fetch under the hood, but fetchNextPage passes cancelRefetch: true as options:

https://github.com/tannerlinsley/react-query/blob/de282541591a7c350035a2fb5c27f5404c5daffa/src/core/infiniteQueryObserver.ts#L93-L103

you can achieve the same behaviour by passing cancelRefetch to refetch:

refetch({ cancelRefetch: true })}

it will make sure that currently outgoing requests are cancelled and the new request will be fired immediately.

I can't really tell why cancelRefetch can't be passed to fetchNextPage (it's hard-coded to true) or why the defaults are different...

[lgenzelis]

Thanks a lot for your answer @TkDodo ! This is interesting, the fact that it is hardcoded this way gives the impression that this is not a bug at all, and that it behaves this way for some specific reason. But I can't imagine what the reason may be. To me false would be a far more sensible default. Moreover, since cancelRefetch: true is enforced (rather than just a default) makes me think that using cancelRefetch: false would break something 😬😬
Let's see if @tannerlinsley remembers the rationale behind this.

[TkDodo]

Even though the defaults are different, we could at least respect the options.cancelRefetch flag when it is passed in, likely doing something like:

cancelRefetch: options?.cancelRefetch ??true,

I would accept a PR if you want to contribute :)

[TkDodo]

Unless of course there is a good reason I’m not aware of 😅

[Seacant]

This actually explains the behavior that's causing the bug I was just about to submit!

I have somewhere in my app where I need to both refetch previous data (if stale) and fetch a new set. When I have

if(query.isStale) {query.refetch()}
query.fetchNextPage()

In console, I can see both network requests succeed and return, but the previous page of data (the one fetched by the refetch) was not actually committed to the query object (ie, data.page[0] was not updated with the new data). If the fetchNextPage call was silently cancelling the previous refetch, that would explain why.

[tannerlinsley]

I think the solution here is:

• Fix the cancelRefetch option to be overridden in infinite refetcher functions. The default should stay the same for now.
• Update and educate on the design decisions as they are now in the docs
• Put a pin in the discussion of what the defaults should be in a future major breaking change

Thoughts?

[TkDodo]

@lgenzelis would you like to contribute that?

[lgenzelis]

Sure would! This will be my first time digging into react-query codebase. I'll give it a try!

[lgenzelis]

Well, here it is: #2731 ! 😁 Take a look when you have a minute please.

[tannerlinsley]

🎉 This issue has been resolved in version 3.26.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀