RTK Query: Conditionally skip the query from the endpoints definition for hooks

[SYoder1]

Is there a way to set a condition or even a callback on a endpoint to conditionally skip it, instead of at the hook using skip or skipToken?

I have a few queries in our app that we should only make if the user has a specific permission. These queries are all around our app, and currently we are having to add logic to ever screen in order to set the hook to skip the query. This is some unnecessary boilerplate code for most queries, and we have thought about making a wrapper hook to extend the skip logic to include the permissions. Is there a better way?

[markerikson]

No, there is currently no way to skip a query from within the endpoint definitions.

It's a thought that's come up a couple times, but it's never been something we've seriously tried to figure out.

Wrapping the hooks with a custom hook is probably the right answer here.

[SYoder1]

Ahh bummer.

I would be open to looking into it and creating a PR if that something you would consider and or would want.

I spent some time looking into the hook builder, and though about being able to pass a selector in the endpoint definition that returns a boolean. If it's true, skip the call like how it checks for the skipToken. https://github.com/reduxjs/redux-toolkit/blob/master/packages/toolkit/src/query/react/buildHooks.ts#L1085

[markerikson]

Conceptually, this would probably need to be integrated into the existing condition method, which is already what we use to bail out of executing the thunks in a number of cases:

redux-toolkit/packages/toolkit/src/query/core/buildThunks.ts

Lines 526 to 573 in aaeda15

	condition(queryThunkArgs, { getState }) {
	const state = getState()
	const requestState =
	state[reducerPath]?.queries?.[queryThunkArgs.queryCacheKey]
	const fulfilledVal = requestState?.fulfilledTimeStamp
	const currentArg = queryThunkArgs.originalArgs
	const previousArg = requestState?.originalArgs
	const endpointDefinition =
	endpointDefinitions[queryThunkArgs.endpointName]
	// Order of these checks matters.
	// In order for `upsertQueryData` to successfully run while an existing request is in flight,
	/// we have to check for that first, otherwise `queryThunk` will bail out and not run at all.
	if (isUpsertQuery(queryThunkArgs)) {
	return true
	}
	// Don't retry a request that's currently in-flight
	if (requestState?.status === 'pending') {
	return false
	}
	// if this is forced, continue
	if (isForcedQuery(queryThunkArgs, state)) {
	return true
	}
	if (
	isQueryDefinition(endpointDefinition) &&
	endpointDefinition?.forceRefetch?.({
	currentArg,
	previousArg,
	endpointState: requestState,
	state,
	})
	) {
	return true
	}
	// Pull from the cache unless we explicitly force refetch or qualify based on time
	if (fulfilledVal) {
	// Value is cached and we didn't specify to refresh, skip it.
	return false
	}
	return true
	},

Honestly, it might even only be a 3-4 line change. Don't destructure getState, then add:

if (endpointDefinition.condition && endpointDefinition.condition(arg, options) === false) {
  return false
}

[SYoder1]

I can play around with that and see how it works.

Would that still keep the hook in a isUninitialized state?

[markerikson]

It should, yeah, because no actions get dispatched at all.

[phryneas]

Keep in mind that this will mess up the TS types a lot, which is the main reason we haven't done this yet.

[SYoder1]

I spent some time on this today, and maybe I wasn't setting this up correctly, but as soon as the hook renders it is in a loading state. I tested this in buildThunks by calling the new function, as well as just adding a check for the endpoint name and returning false at the top of the function. Both ways still keep it "loading"

console.log(isUninitialized, isLoading, isFetching, isSuccess, isError)
// false true true false false

If I add skip: true to the hook, none of the buildThunks code runs (expected), and isUninitialized is still true

console.log(isUninitialized, isLoading, isFetching, isSuccess, isError)
// true false false false false

[markerikson]

Ah, yeah, we do intentionally have an override selector that says "if it's uninitialized return loading: true on the grounds that the request will kick off right after this". So the actual cache entry isn't changing, but we overwrite the derived boolean flags on the way out.

[SYoder1]

Would it be expected to keep it in a loading state for this use case?

[markerikson]

Well, this is where the increasing number of options and behavioral interactions causes increasing complexity :)

The selector code has been able to make this optimization based on a valid assumption that "this thunk will be dispatched in an effect right after this, therefore we can safely assume that the cache entry will go to a status of pending, and we can save a re-render by preemptively overwriting the derived flags with isLoading: true".

If we now have a condition for the thunk, that assumption no longer holds true - it's possible that the effect will run but the thunk will bail out.

So, what's probably needed is to extend the logic in the pre-selector to be along the lines of if (entry.isUninitialized && !endpointDefinition.condition) {.

In other words, it should keep the existing isLoading override for endpoints by default, but if the endpoint does define condition, then we have to skip this flags optimization.

Also, I just glanced at the code. noPendingQueryStateSelector is currently defined at the top level of buildHooks.ts, but in order to have access to the current endpoint it would need moved inside of buildHooks() - see queryStatePreSelector as the similar example.

I was going to suggest moving it right after queryStatePreSelector, but the way we've got the logic inside of buildHooks defined is tricky - there's a bunch of function declarations that get hoisted, but noPendingQueryStateSelector is a const, so it doesn't hoist. Also, there's no good way to give it access to the current endpoint name without some further refactoring.

It's feasible, but it'll take some shuffling of the code.

[SYoder1]

@markerikson maybe I still don't fully understand the nuances of it but I just added the check to the same condition for the skip and skipToken. It's still a draft MR, for the idea and still needs some work around documentation.

[SYoder1]

Hey @markerikson, if you have a second, could you take a look at the MR and let me know what you think?

[markerikson]

@SYoder1 I won't have the mental space available to look at that until after I get the infinite query feature shipped. (Fortunately, that's getting fairly close!)