Mutation Cache Duration

[TkDodo]

As discussed in this discussion, the MutationCache is currently ever growing. This can be a problem because mutations are not shared like queries are, and are also never garbage collected. If mutations return big responses, those will be held in memory indefinitely.

Proposed solution

Add a cacheTime to MutationCache, and default to 5 minutes (same time as for queries). We can't remove mutations from the cache instantly after the last observer unmounts, because callacks on useMutation might fire after that.

For that, we also need to notify the cache when an observer is added or removed for a mutation. The code for scheduling garbageCollection could be shared between queries and mutations via a base class that both Query and Mutation could extend.

[TkDodo]

@Noitidart FYI, given that our discussion sparked this issue. Let me know if you want to maybe work on this

[Noitidart]

Thanks! I'm currently trying to work on another library called valtio-persist, its alterantive to unmaintained redux-persist.

But for this, I was maybe thinking maybe a solution instead of duration cachce, just making it so mutation callbacks fire even if not in cache. So this would be made untrue:

We can't remove mutations from the cache instantly after the last observer unmounts, because callacks on useMutation might fire after that.

[TkDodo]

you can certainly look into it, because it would otherwise mean that if you explicitly set cacheTime: 0, the callbacks wouldn't work as expected.

However, I think having a cacheTime is necessary. Mutations can be paused (e.g. when offline) and stored to localStorage / asyncStorage, where they can then be resumed once the comes back to the app and goes online again. This only works because we keep the mutation state around. If people use that, they will likely want to set a higher cacheTime. So we can't just remove them from the cache if the last observer unmounts without providing a cacheTime setting to work around this.

[Noitidart]

Oh that's super interesting. In that case, where mutation never went through due to being offline, it shouldn't get affected by cache time. Cache maybe we can say applies to the "result", and if an offline mutation has not yet "resulted", it should continue to stay.

[TkDodo]

where mutation never went through due to being offline, it shouldn't get affected by cache time.

it's an interesting idea, but I think if you want that, you'd just need to set a higher cacheTime. Creating an exception for paused mutations is something we can think about though

[Noitidart]

Thanks for this great design discussion!

Even if not paused though, what if cacheTime is 5sec and the internet is slow and the mutation responds at 10sec mark, so then no callbacks would fire. So maybe not just if paused, cacheTime shouldn't apply until (1) first response (2) or until after retry strategy is exhausted?

[TkDodo]

cacheTime only starts to tick after the last observer unmounts, because it’s meant for garbage collection

[Noitidart]

cacheTime only starts to tick after the last observer unmounts, because it’s meant for garbage collection

Oh so awesome! I didn't know this! What happens in case of a paused mutation (due to like offline), does it not start ticking until it has been unpaused and request has gone through?

[TkDodo]

What happens in case of a paused mutation (due to like offline), does it not start ticking until it has been unpaused and request has gone through?

Since the mutation is still in loading state when it’s being paused, the garbage collection will be deferred. If you’d like, you can add a test case for this?