changes to onSuccess callback

[TkDodo]

Discussed in #2911

^{Originally posted by phatmann November 11, 2021}
Calling QueryClient.setQueryData method triggers the onSuccess callback. For many users, including me, this is unexpected behavior, and can be the source of recursion issues, since the onSuccess handler might make calls to setQueryData.

I propose we create a new onDataChanged callback. This would be trigged when cache data is changed via setQueryData or via a fetch. The onSuccess callback would only be called after a successful fetch.

For completeness, the same onDataChanged callback would be added to QueryCache.

[TkDodo]

@phatmann let me know if you want to work on this, too :)

[TkDodo]

talked with @tannerlinsley about this today, and we settled on the following:

• onSuccess should only be called when a fetch is made, basically tying it to the promise. This is what most people expect and why this change is breaking.
• we're not going to introduce another callback. The observers will be informed anyways when data changes, and it would very likely have to be a side-effect during render in the observer in cases where we never even fire request because we read from the cache, which means we would need to implement it with useEffect in useBaseQuery anyhow. We're going to communicate that the best way to trigger a side-effect is, in fact, a useEffect in user-land. Since data returned from useQuery is stable and structurally shared, you'd just need:

const { data } = useQuery({ queryKey, queryFn })
useEffect(() => mySideEffectHere(data), [data])

[phatmann]

This 100% makes sense. Great call.