Open lanther opened 6 years ago
@kerrishotts @chnicoloso @mcav FYI
Awesome stuff here, and definitely something we need to document in the docs (I'll work on that next week).
A few quick thoughts re: persistence:
@Store{persistenceProvider: LocalStoragePersistence}
or similar... just a thought that popped in my head...I'll think about this stuff some more tomorrow when my brain cells are a bit more awake... ;-)
This is fantastic and very clear. Thank you Michael!
While twist makes it easy to persist stores, through having
toJSON()
andfromJSON()
methods, it's easy to run into pitfalls due to persistence having side effects. We need to collect some best practices, and if possible prevent some of the most common issues.Pitfall 1: Overloading the constructor/initialization
If you want a store to load its data from local storage, the following might seem like a good idea:
However, this is bad (other than the lack of try/catch) for two reasons:
It won't work if
MyStore
is nested inside another store, due to the way@State.byRef
works - it first creates a newMyStore
and then callsfromJSON()
to initialize it. So your initial data will be overriden by whatever you passed into the main store.Even if it did work, it would violate the principle of "determinism" - all actions should be deterministic, so that you can replay/debug them. If they're not deterministic, you'll get a different answer each time you run it (or if you run it in different contexts), which makes it really hard to debug. It's the same reason why reducers in redux must be pure functions.
Instead, there are two "correct" ways to initialize your store from local storage data:
You can pass this in, in the top-level JSON for your main store (i.e. when you call
this.scope.store = new MainStore(INIT_DATA)
. This is deterministic, because the INIT action is self-contained - you've already read the data from local storage before initializing the store with it.You might not like the first approach, because the top-level app needs to know where to load the data from. So, a good alternative is to define a
LOAD
action on the store, like so:Now, in the initialization code for the store, you'll do something like:
Note that since
LOAD_FROM_STORAGE
is asynchronous, you need to dispatch it directly on the store in question.Pitfall 2: Non-deterministic actions, and actions with side effects
You might be wondering why the above example has two actions, rather than just one. It's again because of the principle that actions should be deterministic. Think about what would happen if you just wrote:
When you run this through redux devtools, you'll just see a single action "LOAD_FROM_STORAGE" in the logs. But if you were to undo, and then replay this action again, you'd get a different answer because local storage might have changed.
For this reason, you always want to separate anything that's non-deterministic, or has side effects, into an asynchronous action, and keep the actions deterministic.
The same thing applies when you save the store to local storage:
Here, the action is deterministic, but it has side effects (saving to local storage). This will work fine with redux-devtools, because of the determinism, but it'll still cause you trouble if you undo/replay actions while debugging, because this will change the local storage as well. That could lead to you scratching your head... so just keep life simple, and restrict the side effects of actions to the store and its children.
As before, if we split it into a synchronous/asynchronous action pair, all is good:
Possible API Extensions
It's worth looking into adding functionality to Twist, to manage persistence layers. This would need to know:
The neatest solution might be a subclass of Store - e.g. we can have one for LocalStorage, but there could be other types of persistent stores. If so, (1) could be options on the decorator (
@LocalStorageStore({ ... })
) and (4) could be a new method (e.g..load()
), that knows to recurse down the store tree.(3) is a tricky one, and I feel we need to think more deeply about this.