Always call onSuccess even with deduping?

[Linksku]

Bug report

Description / Observed Behavior

In some scenarios, I'd expect onSuccess to be called, but it doesn't. E.g. a route "/foo" populate the state inside SWR's onSuccess. If I quickly navigate /foo -> /other -> /foo before the request from the first /foo finishes fetching, the onSuccess from the second /foo won't run. The result is that the state in the second /foo remains the initial state.

Expected Behavior

Since the first /foo already unmounted, I'd expected the onSuccess from the second /foo to run. Alternatively, there should be a way for configure SWR so onSuccess always runs, even if the request was deduped.

Additional Context

SWR version: 1.0.1

[shuding]

Thanks for opening this! By definition onSuccess and onError are only called with actual requests, because there’re just too many deduplicated revalidations, so these give the “global” events. But there’s another onDiscarded callback for deduped requests (added in latest beta in #1523), I think you can use it together with onSuccess to get the “local” loading state.

[Linksku]

Thanks for the suggestion! I updated to 1.1.0-beta.6 and tried onDiscarded. It looks like onDiscarded is only called if shouldStartNewRequest is true. I can make shouldStartNewRequest false by disabling deduping, but if I disable deduping, then I don't need onDiscarded in the first place.

I feel like this is a common enough use-case that there should be a separate callback that always gets called, even if it was deduped.

[shuding]

I realized that I misunderstood your original message and it’s indeed a problem. Will figure out a way to fix it!

[michaelwschultz]

Running into this issue still. Any update on when or how it might be resolved?

[mko4444]

@shuding is there any update on this? I'm getting a similar issue here and it's breaking my whole app.

[MarkoIvanetic]

Issue is still happening. code sandbox to reproduce:
https://codesandbox.io/s/sweet-benz-ib4krx?file=/src/App.js

[isBatak]

React Query claims that onSuccess and onError are abd API design https://tkdodo.eu/blog/breaking-react-querys-api-on-purpose#a-bad-api and their plan is to remove those callbacks. Maybe SWR should do the same?!
cc @shuding

[BertrandBordage]

I faced the same issue.

In short, the solution for me was to disable deduping globally. I put this inside my <SWRConfig value={…}>:

// Disables deduping. Mandatory for requests that modify data, otherwise we can end up
// with broken frontend states when users are interacting quickly with the application.
dedupingInterval: 0,

In my case, there is no valid case for deduping. The "read" requests I do are cached and rarely revalidated, but the "write" requests can happen in a quick succession. An example is that you can log in, log out and log in again in less than 2 s, the default dedupingInterval. In my case, it lead to a state where it was impossible to log in again, unless you did a full page refresh.

I honestly think that deduping should be disabled by default, as it can be very misleading. Caching is already enabled by default, for most users it will be enough.

I do not have an strong opinion on whether onSuccess should be called when deduping. There could be harmful consequences with both behaviours.

Note that I tried applying @isBatak’s suggestion of using useEffect, but it is not reliable. onSuccess/onError should be kept. The problem with useEffect is that there is no universal way of checking that a query just occurred. In my case, I had a useMutation call to a /logout API endpoint that is returning true when the logout is successful. If you use something like:

useEffect(() => {
  if (data) {
    actualLogoutLogicLikeClearingAnInMemoryToken();
  }
}, [data]);

then once a first logout was triggered, data will always have the same value and never run again the useEffect callback, which also leads a broken state. There can be mitigations to clear data once the useEffect callback was called, but it is unnecessarily complicated and fragile.

[joshkel]

@BertrandBordage - From our perspective, we rely on deduping fairly heavily. For example, three different components that are all mounted at different places in the component tree might need the same information; it's convenient to assume / rely on deduping and treat useSWR as a generic "ensure that this information is available", without having to worry about this generating extra requests.

If I understand your issue correctly, the rapid sequential requests are getting deduped even though there are mutation calls interspersed? If so, that sounds like it may be worth raising as a separate issue - mutation should override any dedupe interval.

[BertrandBordage]

@joshkel Yes, it's somewhat related to mutations, but not necessarily. It's related to queries that modify data on the backend, so roughly POST/PUT/PATCH requests, which can be done with useSWR, even though it's a bad practice.

In my case it's always through useMutation calls that I do POST/PUT/PATCH, so your approximation would work. I will open a new issue, then.

[BertrandBordage]

Sorry that I went slightly off topic… The problem I faced was not onSuccess not being called nor useSWRMutation being deduped.

It was a bug with the way the cache gets "cleared" that dedupes useSWR calls, returning undefined and of course not calling onSuccess. Full report and Codebase example here: #2877

[mikokofuyu]

I have also encountered the same issue where if a user does a back button journey to the page within the 2 second deduping interval then onSuccess is not called.

Is there any other alternative onFunction we can rely on to avoid using a useEffect to update state based on the response?

[15MariamS]

It was a bug with the way the cache gets "cleared" that dedupes useSWR calls, returning undefined and of course not calling onSuccess.

I'm also running into a similar issue where I'm calling an async fetch in the onSuccess of my SWR call but the onSuccess gets passed as undefined. If anyone has figured out a good solution that doesn't require useEffect, I'm keen to learn more.

[tomprats]

It sounds like there should be a separate option instead of onSuccess, maybe you could pass onData to useSWR that would get called like onSuccess, as well as when it loads the data from the cache.