The refetchOn[Event] options should accept a function

[jcready]

Just like with retry accepting a function, so should these.

The use case is I want to disable refetching when the query fails and I want to be able to do this by setting the QueryClient defaults. I want previously successful queries to (by default) refetch on focus. I have no interest in queries that previously failed being refetched every time the user comes back to my window and trigger the exact same error.

The only way to accomplish this today is to const refetchOnWindowFocus = useState(true) next to each useQuery call, add an onError and then change the state.

[TkDodo]

yeah I think that makes sense. I can look into making more options accept a function, or do you want to? For the use-case you are describing, error boundaries also seem like they could do the job nicely, as they will actually unmount the query and thus it will not be refetched. Once the user resets the error boundary, it would start fetching again.

Maybe the first thing to look at would be the enabled option? disabling a query while it has an error seems like a catch-all solution, or am I missing something?

[TkDodo]

@jcready I would be okay to do this now. what would be the best API in your opinion. I'm currently leaning towards:

  refetchOnWindowFocus?:
    | boolean
    | 'always'
    | ((
        data: TData | undefined,
        query: Query<TQueryFnData, TError, TQueryData, TQueryKey>
      ) => boolean | 'always')

so passing only data and query in, and error can be retrieved via query.state.error. But since one use-case is also to turn it off if there is an error, maybe it would be better to expose error as well?

I'm just a bit worried here that every function that we do has a slightly different interface. What do you think?

[TkDodo]

we can also try and streamline the apis, as long as v4 is still in beta. currently, we have:

• refetchInterval: takes data and query
• useErrorBoundary: takes error and query
• retry and retryDelay: takes failureCount and error
• the new refetchOn... functions

I think its okay though that they are different. you can get everything from the query parameter, and some are geared towards data (like refetchInterval, as you'd want to fetch until some data is returned), and some are geared towards errors (e.g. show a boundary when you have a certain error, or retry until you have a certain error...)

retry doesn't take the query, but we could do that on demand, as it's non breaking. I'm not sure what the refetchOn functions should be geared towards though, and I don't want (data, error, query) as an interface I think ...

[Moshyfawn]

Well, I'm thinking about it this way: the retry is definitely an error handling option, which "leans" towards having the error as it's main focus.

The OPs suggestion is coming from having the retry option as an example; the issue they want to solve is around handling the query error to stop re-fetching from happening. So, I'd say the solution to this feature request would be an error-centric API like

(error, query) => boolean

Additionally, I can't really think if a use-case, where I'd disable the re-fetching on a specific data piece I receive from, say, an API.

Now, here's an API design idea just because I had to work with a lib recently that does this pattern, I'm about to describe, a lot.

As there're multiple event types the RQ can execute the re-fetch on, consider the following API (FYI, I'm thinking up the event types as I go):

refetchOn: (eventId: "windowFocus" | "staleData", query: Query<TQueryFnData, TError, TQueryData, TQueryKey>) => boolean

This way the option would be considered more neutral in terms of "leaning towards data, error etc.", which makes it a greater candidate to have a query as it's second argument (after the event type).

I'm not saying it's a great pattern. I'm providing it as an example to give you some prospective on my thought process around the refetchOn[Event] API.

[TkDodo]

it's done: https://github.com/tannerlinsley/react-query/releases/tag/v3.37.0