Is there a way to clear "revertState" ?

[LoicDavid]

Hello,

The question is in the title, but I will give some context.

I was doing performance measures on our webapp and I spotted the following issue:

When fetching data from our server, the RAM used by the page increases (nothing surprising here).
When refetching the exact same data, the RAM increases again (it shouldn't as the exact same thing is displayed).
Then, if I fetch again, it does not increase anymore.

After some investigations, I found the cause. The previous data are kept in the revertState attribute of the query.
After the first query, there is not previous data, so we just use the RAM we actually need. But after, we will use more RAM as the previous data (not actually used in the page) are kept in memory.

We were first using v4, where revertState is a public attribute that can be cleared manually. I did it to check if the memory increase really came from here. However, it was a "dirty" fix we don't want to have in prod.

I updated to v5 to see if this problem was resolved.
It was not (RAM increase still here), and, revertState is now a private attribute (which make even the "dirty" fix impossible).

The fact that revertState is never cleared is a problem for us as we can receive large amounts of data from a single query. Having twice these data in memory can really downgrade the performances.

However, before openning an issue, I prefer asking if there is a way to solve this problem.

[TkDodo]

I think we should re-set revertState when a query finishes successfully, so here:

query/packages/query-core/src/query.ts

Lines 606 to 607 in dfbda9d

	case 'success':
	return {

would you like to open a PR ?

[LoicDavid]

Hey,

Thanks for the answer !

From what I understand of revertState, I totally agree.
However, I'm not sure it is the best place to do it. This switch case is in a reducer. Usually, reducers only return a new value for the variable received in param (in this case state) and should not have side effects.

I guess it could be done:

• When calling #dispatch with the action type 'success', in the setData method. The risk is to forget to do it if, in the future, a 'success' is dispatched from somewhere else.
• In the #dispatch method but with another reducer dedicated to revertState that would return undefined for action type 'success', else, the value received in param.

What do you think ?

I have forked the repo and will open a PR soon to add this quick fix. But, I think it might be interesting to go a bit further.

Now, when fetching, the state is always saved in revertState to have a fallback if the request fails. But, in some cases, we might not want to go back to the previous result anyway, we might prefer having no result.
Thus, there might be an option to totally deactivate this mechanism and always keep revertState undefined (saving memory while fetching).

I will not open a PR for this last point, this is simply a suggestion. ;)

[TkDodo]

The reducer is local to dispatch, so I think it’s fine:

query/packages/query-core/src/query.ts

Line 651 in dfbda9d

this.state = reducer(this.state)

we could also add, right after that:

if (action.type === 'success') {
  this.#revertState = undefined
}

but it’s just more code and detaches it from the other things that happen in case 'success'. I don't think we have to be idiomatic about reducers not having side-effects here. Again, it's just a local implementation detail of #dispatch.

[LoicDavid]

Pull request opened here: #9265