Refactor state mgmt #1326
Refactor state mgmt #1326
Conversation
|
Any reason the beta deployments failed? |
|
@ZeldaZach I don't know if you noticed but this is just a first step. Not all the data is being handled by redux stores. |
|
Actually, I just read : https://dev.to/selbekk/persisting-your-react-state-in-9-lines-of-code-9go I need to think about it a bit, because we may not need actual global state, I need to check again but if we can get along with just local state + persistance we could just be happier. @ZeldaZach I don't know who could have an expert POV on this subject, if you know someone let me know. |
|
cc @lrawicz |
|
@HerveH44 I'm keen on redux. In particular being able to write tests around certain bits of logic (like what should happen when you burn a card you've previously picked?) ITO using local storage and hooks isn't that just global state but somewhere else and harder to test? Very excited to see this as part of the app, I think it will make unbeatable the so and contributing a lot easier. Happy to help out with this where possible |
|
@mixmix thanks, I'll be glad to share thoughts on Redux. I've never put frontend under test, so I would love to know more about it. This PR intent is to be super small and just starts the migration to Redux. I already started with another part of state but it requires a lot of changes. @ZeldaZach @mixmix if you think this is fine, we can merge this part and move towards another part of state refactor |
| @@ -0,0 +1,31 @@ | |||
| const readAndDeleteFromLocalStorage = (key, defaultValue) => { | |||
| const val = localStorage[key]; | |||
There was a problem hiding this comment.
Should probably use localStorage.setItem
getItem and removeItem?
I guess that protects against accidentally mutating wrong things on the raw object?
| } | ||
| } | ||
| } | ||
| ); |
There was a problem hiding this comment.
These reducers are what we could easily write tests over. Haven't seen this createSlice thing before but we just need a way to reach in and check those reduce functions. These ones are simple, but would be good to get into habit of putting tests over ones which have multiple paths over them.
I would say we should have a testing pattern in place before merging this. Would set us up well. Happy to help
|
I'm keen to see redux put in place. The app starts is kinda stressful currently. This PR seems like a great start. Do we also need to consider how any recent changes to react have influenced this work? |
Linked tickets
Explanation of the issue
The state management is a big mess and we should adopt a simpler and more maintainable way of handling frontend app state
Description of your changes
I'm starting small to allow incremental changes. I just introduced Redux store capability and connect the store to StartPanel controls.
I added migration capability to make it invisible to the user that the data is being migrated to the new store.
Screenshots