Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

resetStore not clearing store! #1849

Closed
stantoncbradley opened this issue Jun 30, 2017 · 14 comments
Closed

resetStore not clearing store! #1849

stantoncbradley opened this issue Jun 30, 2017 · 14 comments
Labels

Comments

@stantoncbradley
Copy link

Intended outcome:

resetStore clears the Apollo data

Actual outcome:
resetStore only refetches, there is no point where data in components is cleared

How to reproduce the issue:

error repo: https://github.com/stantoncbradley/apollo-reset-store

  1. Run the app :)
  2. Clicking resetStore button triggers Apollo resetStore
  3. App logs nextProps as the come in through componentWillReceiveProps
  4. At no point in the logs is the data cleared

Our actual use case is a user signing out of our app. We noticed if another user logs in and one of the queries fails, data from the last user displays! It was never cleared!!!!

@helfer
Copy link
Contributor

helfer commented Jul 1, 2017

@stantoncbradley thanks for filing the issue along with a reproduction and good description of what's going on.

@jbaxleyiii FWIW: I think the data may be cleared in the store, but due to this PR in react-apollo, the query data isn't read from the store, but from here. Maybe resetStore should somehow also clear the lastResult for each observableQuery, but things get complicated when you go down that path.

@helfer helfer added the 🐞 bug label Jul 1, 2017
@stantoncbradley
Copy link
Author

@helfer no problem! A couple things on further investigation:

  1. Our issue seems to have been introduced in react-apollo 1.2.0, as 1.1.3 doesn't show our previous user's data but shows the empty data.
  2. Testing my repo app with react-apollo 1.0.2 never passes down cleared data props to the component either, it just replaces the data with the latest fetch.

Perhaps I am unclear on the intended behavior of resetStore. Should it flush out the components with empty data to clear everything and then refetch? Or is it preferable to keep the existing data and replace it if/when new data comes in?

For our signout use case, we want to clear the store and clear the data in all of our components. The docs suggest using resetStore for signout, however, even if data is being cleared from the store, it's not propagating down and clearing the components (testing with 1.0.2 shows at no time does componentWillReceiveProps receive null/empty/cleared data.

The current behavior of resetStore (keeping existing data until refetched data resolves) may have valid use cases (personally we don't have any such use cases yet). However, we definitely have a use case where we want the entire store cleared out and cleared for all our components (namely on signout).

Since resetStore apparently doesn't (and never, at least as of 1.0.2) clears the components data in between fetches, I'm wondering if it would be disruptive to change that functionality now. It seems to me we either need a different API for completely clearing the data store & components(clearStore?), or perhaps a config on resetStore to cleanse the data between fetching (resetStore({ hardReset: true }) or something like that).

For now we are rolling back to 1.1.3 because at least our data is written over with the empty data from the refetch. However, even in 1.1.3 I don't think our data is really being cleared when we call resetStore, because like I said, react-apollo never passes empty data to flush the components before it refetches, so our data isn't cleared, it's just replaced.

Please let me know your thoughts on this, I'm happy to help resolve. Thanks!

@jbaxleyiii
Copy link
Contributor

@stantoncbradley @helfer keeping the data is indeed the intended effect since you may have a mixture of user data and global data. So components showing things that aren't user specific didn't want to re-render until it has new data to show.

@stantoncbradley a couple options here:

  1. When you call resetStore, trigger a rerender of your tree from a wrapper right below ApolloProvider
  2. PR into react-apollo to do that automatically. This would need a PR to allow something like onStoreReset in AC.

I'd recommend trying option one (we should document this as well) since both behaviors are valid.

Does this help?

@stantoncbradley
Copy link
Author

@jbaxleyiii thanks for the suggestion. How do you recommend doing #1?

I tried

  reset = () => {
    this.props.apollo.resetStore();
    this.forceUpdate();
  }
...
<button onClick={this.reset}>resetStore()</button>

in my example repo and I'm still not getting empty data

thanks!

@jbaxleyiii
Copy link
Contributor

jbaxleyiii commented Jul 5, 2017

@stantoncbradley resetStore returns a promise, can you try the force update after that is done?

@stantoncbradley
Copy link
Author

@jbaxleyiii

  reset = () => {
    this.props.apollo.resetStore().then(() => this.forceUpdate());
  }

even added a 5 sec delay to my resolver, still not seeing empty data in between fetches

const getPeopleData = () => {
  const data = [
    { id: 1, name: 'John Smith', time: new Date() },
    { id: 2, name: 'Sara Smith', time: new Date() },
    { id: 3, name: 'Budd Deey', time: new Date() },
  ];

  return new Promise(resolve =>
    setTimeout(() => resolve(data), 5000)
  )
}

image

thanks!!

@stantoncbradley
Copy link
Author

stantoncbradley commented Jul 5, 2017

looking at the code for apollo-client, I clearly see that the store gets reset in the reducer:

} else if (isStoreResetAction(action)) {
    // If we are resetting the store, we no longer need any of the data that is currently in
    // the store so we can just throw it all away.
    return {};
  }

I think @helfer is right, this may be an issue with react-apollo (should I move the issue over there?). digging into that code now to see if I can track down why the empty data store from ApolloClient isn't propagating to the components. Note @helfer the empty data set from resetStore doesn't propagate to my components even in v1.0.2, which was released before apollographql/react-apollo#548

EDIT:
just saw the part about "lastResult for each observableQuery", going to dig around in there...

@jbaxleyiii
Copy link
Contributor

@stantoncbradley yeah lets move this to the react integration! I'll see if I can come up with a solution!

@jbaxleyiii
Copy link
Contributor

Attached to apollographql/react-apollo#807

@7ynk3r
Copy link

7ynk3r commented Jan 3, 2018

This to me is still a bug.

I don't see the point of resetting not resetting anything. This also becomes a problem if you persist the cache, because you do nothing.

If you need to use the data, take it out before resetting or provide another method to do it or an argument.

The only workaround I found is creating a new client, and if you're persisting the cache, purge before.

@stalniy
Copy link

stalniy commented Jan 23, 2018

The way resetStore works is not ideal. In my ionic application I use nav.setRoot('HomePage') after login and logout. This method removes all navigation state, thus my HomePage sends request to fetch data and resetStore sends request to fetch data. Eventually there are 2 duplicated queries which I would like to avoid.

Is it possible to provide at least option to not fetch queries? Or clear all queries, even active, so people who know what they do can do this :)

@jason-shen
Copy link

@stalniy did you find a solution for this? i am facing the same issue, not sure where to go now

@n0ne
Copy link

n0ne commented Mar 16, 2018

+1

@stantoncbradley
Copy link
Author

client.cache.reset() will actually clear the cache
vuejs/apollo#53 (comment)

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

7 participants