Framework: persist redux state

[gwwar]

Persisting Redux State

As part of the offline investigation made in our Miami Calypso Core Meetup, this PR persists redux state using localforage, and loads it as the initial state on boot. This may provide us with some minor performance improvements, and compliments our offline use case. We currently clear this state when users log off.

Blockers

( #2757, #2759 ) the redux tree is currently not "pure". Some sections of the tree contain functions, which means that errors are thrown during serialization. Unless we wish to write custom serializers/deserializers, the redux tree should be cleaned up to ensure that it only holds simple objects that do not contain blobs or functions.

Update: I'm currently working around this by adding in SERIALIZE and DESERIALIZE actions that are triggered during loading/before writing to localforage. These actions are not dispatched normally and will not modify the existing store. It instead modifies the copy of current state before we write to localforage, and the initialState object we use when creating a store respectively.

In each reducer, SERIALIZE takes what's in memory and returns a simple JS object. Similarly in DESERIALIZE the reducer is given state that is a simple JS object. It should validate if the data is valid using a schema. If it is valid, further manipulate the JS object to match what your subtree expects in memory (this might be a no-op, or say instantiation of Immutable.js objects). If the data is invalid, return the default initial state.

Open questions

• How do we deal with loading stale or malformed state? TBD
• Should we allow sections of the tree to use immutable state? ( We'd need to create immutable JS instances for those sections ). Yes, this is allowed provided that the sub-tree handles SERIALIZE and DESERIALIZE correctly
• Should we allow cycles to exist in our redux state? No
• What common conventions should be enforced between each sub-tree? TBD

Action Items

• In an effort to limit the size of this PR, I will return default initial state for both SERIALIZE and DESERIALIZE for many sub-trees. I will create an issue for each piece of the subtree to implement persistence.
• Create issue to move over calls to localStorage and store over to localForage. (Do we run into any specific storage issues if those other keys are on indexDB vs localStorage?) ( Framework: use localforage instead of store or localStorage #2972 )
• Lots more tests for bad data cases

Subtree Status

• state/application (connection status) - Never Persisted
• state/current-user - Persisted
• state/notices - Never Persisted
• state/plugins - (Safe to merge). Persistence needs to be implemented ( Framework: implement redux persistence for state/plugins #2967 )
• state/posts - (Safe to merge). Persistence needs to be implemented ( Framework: implement redux persistence for state/posts #2968 )
• state/sharing/publicize - Persisted
• state/site-settings/exporter - (Safe to merge). Persistence needs to be implemented. ( Framework: implement redux persistence for state/site-settings/exporter  #2969 )
• state/sites/plans - (Safe to merge) Persistence needs to be implemented ( Framework: implement redux persistence for state/sites/plans #2970 )
• state/sites - Persisted
• state/support - Never Persisted
• state/themes - Persisted
• state/ui - (Safe to merge). Persistence needs to be implemented. ( Framework: implement redux persistence for state/ui #2971 )
• state/users - Persisted

Testing instructions

• There shouldn't be any visible changes to the site, and we should be able to walk through our normal flows without errors. You should see new debug messages like the following each time a page is loaded:
  
• After a user logs out, the 'redux-state' should be cleared from indexDB.

Persisted sub-trees to test, with pings for folks who have the most changes in the reducers:

• state/themes: http://calypso.localhost:3000/design works as expected ( @ehg @ockham )
• state/current-user and state/users: loads OK on page load ( @aduth )
• state/sharing/publicize: Start a post that has a publicized connection, reload the editor page, persisted data should load without validation errors. ( @aduth )
• state/sites: Navigate to 'My Sites', and switch to a few different sites. Attempt to reload the page. Persisted data should load without validation errors. ( @drewblaisdell )

cc @rralian @mtias @retrofox @artpi @aduth @dmsnell

[dmsnell]

ack! functions in the Redux tree!

[aduth]

Some sections of the tree contain functions, which means that errors are thrown during serialization.

Yeah, these should probably be refactored to not contain functions.

[dmsnell]

Considering how central this loader will be I think it would be really helpful to declare the handler function outside of the call to getItem(). We should definitely have tests to handle bizarre cases here.

On that point, I think we could also benefit from using the Promise interface to localforage because it would allow us to build up more robust error-handling without mixing the logic with our successful data handling.

local forage.getItem()
    .then( loadInitialState )
    .catch( handleStateLoadError );

[gwwar]

Definitely. I was also thinking that each module should also verify the initial state that gets passed to it, and add related tests in each module.

[blowery]

A couple things that spring to mind here too:

1. How does this handle Immutable instances? Can it / should it?
2. How does this handle cycles in the object graph? JSON can't have them, but redux state certainly could.

[dmsnell]

This part doesn't look too scalable here. We could mimic the whole reducer tree here with an initialization/pruning tree, which could bring about the same scalable unity of the state itself.

const pruneSites = () => undefined; // could be more intelligent here
const pruneThemes = () => undefined;

const pruneState = state => Object.assign( {}, state, {
    sites: pruneSites( state.sites ),
    themes: pruneThemes( state.themes )
} );

or we could make a second iteration here to abstract it further

import { fromPairs } from 'lodash';

const pruners = {
    sites: pruneSites,
    themes: pruneThemes
};

const pruneState = staleState => {
    const prunedState = formPairs( Object
        .keys( pruners )
        .map( key => [ key, pruners[ key ]( state[ key ] ) ] ) );

    return Object.assign( {}, staleState, prunedState );
};

[gwwar]

Ideally we would remove the deletes when the redux tree is tidied up, but we can certainly employ something smarter if we only wish to persist portions of the tree.

[dmsnell]

my point here was mainly to suggest separating out the delete-logic from the loading logic because it seems like we could end up with a mess here if more parts of the tree need to be deleted. in my recommendations above, we would simply replace those delete lines by this call:

let state = Object.assign( {}, reduxStore.getState() );

state = pruneState( state );

Now we can go on and do whatever we want to prune, but it won't interfere what's happening here. Also, as we get the ability to remove those deletes, this code won't be affected

[aduth]

FYI, @dmsnell, to your code sample above, we should not be importing from Lodash this way, as it increases each bundle size by about ~500kb. See #735 for a detailed explanation.

[gwwar]

Thanks for the pointer @aduth. @dmsnell it doesn't look like fromPairs is available in 3.10.1, we can bump lodash to 4.0.0 in a separate PR.

[blowery]

Wrote up some fiddles to test:
IndexedDB http://jsfiddle.net/6f4jLzp3/3/
WebSQL: http://jsfiddle.net/z8zbc7rj/1/
LocalStorage: http://jsfiddle.net/qgsyp8p9/1/

Indexed DB seems to actually support cycles and custom JS objects, which is interesting.
WebSQL is converting to JSON and exploding on cycles. Immutable comes back as a plain array.
Same for LocalStorage as WebSQL.

[blowery]

One other thing to think about: How do we deal with old state trees? If I use the app today and spin out a state tree to storage, then spin the app up again in a couple months, how do we migrate the state? Or do we just throw it out?

[gwwar]

One other thing to think about: How do we deal with old state trees?

@blowery Good point. We most likely need to version our tree, and bump it each time a breaking data change occurs (not just an additive property). It may be easier to maintain if we do this per module. If the version doesn't match, we build with no initial state.

[dmsnell]

the return kind-of breaks the beautiful chain!

analyticsMiddleware: store => next => action => next( action );

of course, isn't the last part of the chain here unnecessary anyway? I think that the call after next => just returns next

analyticsMiddleware: store => next => next;

Here, next gets returned and if you call the returned result, as it is written, it returns a function that accepts a single argument, which it calls action, then calls next( action ). In my shortened version, it actually returns the next function itself and if you call it again and happen to pass action, it will return the result of passing action into next, which is equivalent.

[gwwar]

👍 updated in 81999f8

[dmsnell]

thanks @gwwar for your work on this. The introduction of localforage raises a few ❓s about our use of things like store and localStorage. This might be the impetus for standardizing to one system. If not, we might come to the point where we realize we're loading bunch of different npm modules, all of which serve the same purpose.

[dmsnell]

We most likely need to version our tree, and bump it each time a breaking data change occurs

This scares me a bit. I left a comment in the code about these kinds of breaks and testing for them. I think we can do better, detecting invalid constructions and handling them appropriately (which could simply be flushing the tree and starting over).

If we start versioning, it could get messy real quickly considering how many changes we make to Calypso in a given week.

[blowery]

If we start versioning, it could get messy real quickly considering how many changes we make to Calypso in a given week

Conversely, if we throw out the state tree on version mismatches, we lose a good hunk of the benefit of having it stored in the first place, for the same reason, our overall version numbers change a ton.

[gwwar]

If we start versioning, it could get messy real quickly considering how many changes we make to Calypso in a given week.

If we choose not to version I would suggest holding redux state for a short amount of time. In my experience dealing with persisted unversioned documents can get messy very quickly.

[dmsnell]

it seems like we could build on the parseTree idea and have each reducer pair with a loader. That loader could pick and choose what to keep from the local cache.

Additionally, that loader could validate the stored copy and determine whether we need to wipe it clean or if we can pick and choose what is already stored. In fact, couldn't we define a global action type, such as VALIDATE_AND_PRUNE which we could then run right after loading? Then, the loader could be built directly into the reducers and that logic would end up staying close to the related logic of the reducer.

[blowery]

In fact, couldn't we define a global action type, such as VALIDATE_AND_PRUNE which we could then run right after loading

I think you want that to happen as part of the load, not after the load, otherwise you'll have a transient bit of bad state in the tree. Or maybe I'm misunderstanding what you mean?

Maybe VALIDATE_AND_PRUNE is what actually does the work of getting the loaded data through the reducer and into the global state? That would work for me...

[blowery]

If we choose not to version I would suggest holding redux state for a short amount of time. In my experience dealing with persisted unversioned documents can get messy very quickly.

All the plusses I have to give to that. 💯 Unversioned docs over time are a nightmare.

[dmsnell]

I think you want that to happen as part of the load, not after the load

@blowery, this might be moot, since as I understand it, redux dispatching is synchronous:

loadCachedState() {
    let state = loadFromLocal()

    store = createStore( state )
    store.dispatch( VERIFY_AND_PRUNE )
}

so I was still suggesting that the sequence stay within the load function, but merely that the implementation of the pruning be distributed among the separate reducers

[rralian]

The change to postpone schema validation to a later PR is a sensible one. I've tested in latest chrome, safari, and firefox and it's looking and working well for me. I've also tested with persist-redux disabled and that's working fine for me as well. I think this is ready to 🚢.

[gwwar]

@blowery @dmsnell @artpi @aduth @rralian @mtias @ehg @scruffian @gziolo Thanks for taking the time to review!