RFC add a CLEAR action #150

Closed
opened 2016-08-04 18:45:52 +00:00 by mattkrick · 13 comments
mattkrick commented 2016-08-04 18:45:52 +00:00 (Migrated from github.com)

Right now we've got LOAD and SAVE, but what happens if a person logs out? Chances are, we'll want a CLEAR.

Thoughts?

Right now we've got LOAD and SAVE, but what happens if a person logs out? Chances are, we'll want a CLEAR. Thoughts?
michaelcontento commented 2016-08-04 19:31:35 +00:00 (Migrated from github.com)

redux-storage simply reflects the state store in your redux application. So if the users logs out your state should already reflect this change, right? And if this is the case the user should be logged out for redux-storage too. As long as the SAVE action has not been prevented (e.g. white- / blacklisting or a fast browser reload in combination with a high debounce time).

Or is there a special case I'm missing?

`redux-storage` simply reflects the state store in your `redux` application. So if the users logs out your state should already reflect this change, right? And if this is the case the user should be logged out for `redux-storage` too. As long as the `SAVE` action has not been prevented (e.g. white- / blacklisting or a fast browser reload in combination with a high [debounce](https://github.com/michaelcontento/redux-storage-decorator-debounce) time). Or is there a special case I'm missing?
mattkrick commented 2016-08-04 22:08:09 +00:00 (Migrated from github.com)

Yeah, I think the alternative is to write a null reducer & call store.replaceReducer(nullReducer). It's not bad, but it's something I'd only ever do if I were persisting the state, so it'd be great to lift that out of the application logic & into the persist logic.

Yeah, I think the alternative is to write a null reducer & call `store.replaceReducer(nullReducer)`. It's not bad, but it's something I'd only ever do if I were persisting the state, so it'd be great to lift that out of the application logic & into the persist logic.
michaelcontento commented 2016-08-04 22:48:11 +00:00 (Migrated from github.com)

But .. uhm .. well replaceReducer is an advanced API that you shouldn't need to bother with - most of the time. Especially for such a simple problem.

I also don't fully understand your need for the null reducer solution.
What is the problem your trying to solve?
In my head the situation looks like this:

  1. Given the user is logged in an the state is something like

    state = { user: { name: 'mat' } }

    • Note: We assume that this state is also currently known to redux-storage
  2. Now the user logs out and a USER_LOGOUT action is dispatched

  3. Someone (UserReducer!?) is triggered by the USER_LOGOUT action and modifies the state

    state = { user: null }

  4. The changed state will be detected by redux-storage and written to the engine

  5. Done

And this behaviour sounds pretty sane to me 😄

But .. uhm .. well `replaceReducer` is an advanced API that you shouldn't need to bother with - most of the time. Especially for such a simple problem. I also don't fully understand your need for the null reducer solution. What is the problem your trying to solve? In my head the situation looks like this: 1. Given the user is logged in an the state is something like `state = { user: { name: 'mat' } }` - **Note**: We assume that this state is also currently known to `redux-storage` 2. Now the user logs out and a `USER_LOGOUT` action is dispatched 3. Someone (`UserReducer`!?) is triggered by the `USER_LOGOUT` action and modifies the state `state = { user: null }` 4. The changed state will be detected by `redux-storage` and written to the engine 5. Done And this behaviour sounds pretty sane to me 😄
jordanh commented 2016-08-04 23:05:40 +00:00 (Migrated from github.com)

In @mattkrick's case, we are using store.replaceReducer() to dynamically load different reducers between routes. We're building a fairly complex app.

When we hit our logout route, it won't be clear what information we want to expunge from the store...but, we want to get it all. We're trying to avoid leaving one user's private information in localStorage and having it available to another if they switch accounts when using the same web browser.

@mattkrick: I think store.replaceReducer(...) with reducer that's built from a whitelist is probably the way to go. I think @michaelcontento's point of redux-storage is just a mirror (roughly paraphrasing) makes sense...

In @mattkrick's case, we are using `store.replaceReducer()` to dynamically load different reducers between routes. We're [building a fairly complex app](https://github.com/ParabolInc/action). When we hit our logout route, it won't be clear what information we want to expunge from the store...but, we want to get it all. We're trying to avoid leaving one user's private information in localStorage and having it available to another if they switch accounts when using the same web browser. @mattkrick: I think `store.replaceReducer(...)` with reducer that's built from a whitelist is probably the way to go. I think @michaelcontento's point of `redux-storage` is just a mirror (roughly paraphrasing) makes sense...
mattkrick commented 2016-08-04 23:06:32 +00:00 (Migrated from github.com)

that definitely works for 1 reducer that you own, but for fun let's say you have 8 reducers: some you own, some are 3rd party, some are loaded initially, some are asynchronously added via webpack magic + replaceReducer.

For example, you have a reduxForm reducer & you persist some form data. If their browser crashes, you are a rockstar because the boot back up to a their form half filled out. That's awesome! But if they click log out, you should probably clear their half-finished form because it has sensitive info & they're at a public computer.

It's not enough to just null out the reducer (state.reduxForm = undefined), rather you'll need to delete state.reduxForm since your initial reducer doesn't know about it.

that definitely works for 1 reducer that you own, but for fun let's say you have 8 reducers: some you own, some are 3rd party, some are loaded initially, some are asynchronously added via [webpack magic + replaceReducer](https://github.com/ParabolInc/action/blob/master/src/universal/routes/welcome.js#L22-L23). For example, you have a reduxForm reducer & you persist some form data. If their browser crashes, you are a rockstar because the boot back up to a their form half filled out. That's awesome! But if they click log out, you should probably clear their half-finished form because it has sensitive info & they're at a public computer. It's not enough to just null out the reducer (`state.reduxForm = undefined`), rather you'll need to `delete state.reduxForm` since your initial reducer doesn't know about it.
mattkrick commented 2016-08-04 23:09:51 +00:00 (Migrated from github.com)

@jordanh whoa same time! yeah, let's just null out the entire thing, & then replace the nullReducer with an initialReducer so it's the same as a refresh.

@jordanh whoa same time! yeah, let's just null out the entire thing, & then replace the nullReducer with an initialReducer so it's the same as a refresh.
michaelcontento commented 2016-08-04 23:10:56 +00:00 (Migrated from github.com)

Well there is always the engine.save({}) sledgehammer 😉

Well there is always the `engine.save({})` sledgehammer 😉
mattkrick commented 2016-08-04 23:15:28 +00:00 (Migrated from github.com)

^^^^ _that!_ ^^^^
let's go with that. sledgehammers get stuff done!

^^^^ **_that!**_ ^^^^ let's go with that. sledgehammers get stuff done!
michaelcontento commented 2016-08-04 23:22:55 +00:00 (Migrated from github.com)

Note that this would only solve a potential information leak through redux-storage. And ONLY up until a new action triggers a new engine.save!

This is important to remember: redux-storage is the "persistence layer behind redux". So if your unable to fully clear/reset the state within redux there is still a possible information leak. And those informations might also be stored through redux-storage at any time in the future.

Only engine.save({}) with a immediate full redux-stack reload (possible by a browser refresh) would solve the information leak on all layers.

Note that this would only solve a potential information leak through `redux-storage`. And **ONLY** up until a new action triggers a new `engine.save`! This is important to remember: `redux-storage` is the "persistence layer behind `redux`". So if your unable to fully clear/reset the state within redux there is still a possible information leak. And those informations might also be stored through `redux-storage` at any time in the future. Only `engine.save({})` with a immediate full redux-stack reload (possible by a browser refresh) would solve the information leak on all layers.
jordanh commented 2016-08-04 23:24:25 +00:00 (Migrated from github.com)

Ha. We're having this same conversation on our internal Slack right now!

Ha. We're having this same conversation on our internal Slack right now!
michaelcontento commented 2016-08-07 10:34:45 +00:00 (Migrated from github.com)

Did you find a final solution you're happy with? If so, would it be possible to share it with us? 😄

Oh, and please close this issue if there is nothing redux-storage can do for you 👼

Did you find a final solution you're happy with? If so, would it be possible to share it with us? 😄 Oh, and please close this issue if there is nothing `redux-storage` can do for you 👼
mattkrick commented 2016-08-07 18:57:19 +00:00 (Migrated from github.com)

yep, all good over here. Thanks for putting up with me while we found a good pattern!

yep, all good over here. Thanks for putting up with me while we found a good pattern!
jordanh commented 2016-08-08 12:58:32 +00:00 (Migrated from github.com)

(deleted our original solution, posting a new one)

Originally we were going to replace our dynamically loaded reducers but in the end went with a much simpler, much more redux-y solution.

After reading this Stackoverflow answer, we ended up wrapping our combined reducers in a new rootReducer that can reset application state.

Now we we log out, we simply dispatch the reset action and let redux-storage act as a mirror.

Details are in this PR from our repository.

(deleted our original solution, posting a new one) Originally we were going to replace our dynamically loaded reducers but in the end went with a much simpler, much more redux-y solution. After reading [this Stackoverflow](http://stackoverflow.com/questions/35622588/how-to-reset-the-state-of-a-redux-store) answer, we ended up wrapping our combined reducers in a new rootReducer that can reset application state. Now we we log out, we simply dispatch the reset action and let `redux-storage` act as a mirror. Details are in [this PR](https://github.com/ParabolInc/action/pull/157) from our repository.
This discussion has been locked. Commenting is limited to contributors.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
MichaelContento/redux-storage#150
No description provided.