Framework: experiment with schemas to validate persisted redux data

[gwwar]

The Problem

If we persist redux data, what happens when our tree changes? Our data shape is different right? Currently we do nothing, which is a little like this:

But as a dev, lets say we run master, let the redux tree persist, and then switch to a branch that has some minor refactoring for an existing subtree.

We then see something like:

Not good. A normal user can hit this case too, just by visiting our website, and the coming back 2 weeks later.

(Don't worry persist-redux is off in all envs!)

Solution

Let's check to see if our data shapes match. If the persisted data doesn't match our schemas, we throw it out and rebuild it from scratch.

As a continuation from #2754, this is an example of how we might use schemas to validate persisted redux data.

As a recap, 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. With this PR, we then validate if the data matches the shape that we expect. If it is valid, we further manipulate the JS object to match what the subtree expects in memory (this might be a no-op, or say instantiation of Immutable.js objects). If the data is invalid, we return the default initial state.

There is some developer overhead for maintaining schemas, but I find that it's a great way to understand what's in the tree (without spinning up Calypso), and it gives us a lot of flexibility in refactoring our tree, since if the shape doesn't match, we rebuild it from scratch.

The following uses state/sites/items as an example using json schema v4 and tv4

Testing Instructions

• In config/development.json set "persist-redux" to true
• Navigate to "My Sites"
• Switch between a few sites
• Reload the page
• Sites should be loaded without errors
• Tests in client/state/sites should also pass

[gwwar]

cc @mtias @aduth @blowery @rralian @dmsnell

[gwwar]

This is somewhat of a tangent, but I also checked what it would take to clean up sites in the redux tree. I ran into a few issues:

1. How do we cleanly pass the redux store to SitesList, where the api requests are occurring?
2. We expect our in memory sites to be class instances of lib/sites/index.js or lib/site/jetpack.js. Should we move away from this pattern?

[aduth]

I also checked what it would take to clean up sites in the redux tree.

It would be nice if we could somehow intercept the received site prior to it being modified in the sites-list module. Perhaps initializing the global instance early in the page lifecycle, and having the list module be aware of the Redux store, dispatching the received site upon being received? This seems admittedly awful as a long-term solution, but the long-term solution is to remove the sites-list altogether anyways.

Otherwise, maybe we should consider performing the site object cleanup from the SITE_RECEIVE reducer handler, so that pages/components making use of sites Redux state don't come to expect these properties and functions to continue to exist.

Should we move away from this pattern?

Yes. If context-specific behavior is desired, these could probably be reimplemented in the selectors and actions, e.g.

if ( isJetpackSite( state, siteId ) ) {
    dispatch( requestAvailableJetpackUpdates( siteId ) );
}

[gwwar]

Otherwise, maybe we should consider performing the site object cleanup from the SITE_RECEIVE reducer handler

This sounds good as a first step, and move where SITE_RECEIVE is being fired for #2757 Some of the sites-list lifecycle changes might be tricky and would be better in a separate PR.

[aduth]

I like the choice of tv4 for schema validation. It's small (~8kb gzipped), has some popularity, and implements a well-defined spec. If anything, there's a notable lack of recent updates, though perhaps this means it's reasonably stable enough to not warrant frequent changes.

[gwwar]

@aduth Thanks for the 👀 ! Great feedback.

[blowery]

how nasty would it be to use a whitelist? tends to be safer and more future proof.

[gwwar]

I was hoping to fix this properly soon by passing the resource before it's decorated to fix #2757

[blowery]

tests pass, trying the code

[blowery]

code works. my comments are just minor things, feel free to 🚢

[gwwar]

Thanks for looking @blowery and @aduth! I'm going to squish commits, and follow up this PR with more documentation on why we need this, and how to implement these schemas.