Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Framework: add separate server state hydration #3343

Merged
merged 1 commit into from
Feb 19, 2016
Merged

Framework: add separate server state hydration #3343

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions client/state/action-types.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ export const RECEIPT_FETCH_FAILED = 'RECEIPT_FETCH_FAILED';
export const REMOVE_NOTICE = 'REMOVE_NOTICE';
export const SELECTED_SITE_SET = 'SELECTED_SITE_SET';
export const SERIALIZE = 'SERIALIZE';
export const SERVER_DESERIALIZE = 'SERVER_DESERIALIZE';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was going to say that BOOTSTRAP may give more clue as to what this is for, but I checked back, and that term is not used at all in the server-rendering section of the Redux docs, so I think it is fine as is.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not attached to the name, so I'd be happy to open up another PR if we find that another term is more clear.

export const SET_EXPORT_POST_TYPE = 'SET_EXPORT_POST_TYPE';
export const SET_ROUTE = 'SET_ROUTE';
export const SET_SECTION = 'SET_SECTION';
Expand Down
19 changes: 11 additions & 8 deletions client/state/initial-state.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@
* External dependencies
*/
import debugModule from 'debug';
import pick from 'lodash/pick';

/**
* Internal dependencies
*/
import { createReduxStore, reducer } from 'state';
import { SERIALIZE, DESERIALIZE } from 'state/action-types'
import { SERIALIZE, DESERIALIZE, SERVER_DESERIALIZE } from 'state/action-types'
import { getLocalForage } from 'lib/localforage';
import config from 'config';

Expand All @@ -24,7 +25,8 @@ export const MAX_AGE = 7 * DAY_IN_HOURS * HOUR_IN_MS;
function getInitialServerState() {
// Bootstrapped state from a server-render
if ( typeof window === 'object' && window.initialReduxState ) {
return window.initialReduxState;
const serverState = reducer( window.initialReduxState, { type: SERVER_DESERIALIZE } );
return pick( serverState, Object.keys( window.initialReduxState ) );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need to pick here? Related, if there's a need to trim reduced state to reflect that of the global object, is there not a concern about nested structures?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I don't, server state will override persisted state with default initial state.

@seear which sections of the tree do you see us bootstrapping, and do we need to be more precise?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently the state tree being bootstrapped is defined by the pick on the server. This works fine for now, but perhaps once the server code becomes more modularized, we can remove the server pick and change this client code to use a formally-defined bootstrap tree.

A reducer that did not return any default state for the SERVER_DESERIALIZE action type would be great for that. Not sure if that is possible.

}
return {};
}
Expand All @@ -34,11 +36,9 @@ function serialize( state ) {
return Object.assign( serializedState, { _timestamp: Date.now() } );
}

function deserialize( localforageState ) {
delete localforageState._timestamp;
const serverState = getInitialServerState();
const mergedState = Object.assign( {}, localforageState, serverState );
return reducer( mergedState, { type: DESERIALIZE } );
function deserialize( state ) {
delete state._timestamp;
return reducer( state, { type: DESERIALIZE } );
}

function loadInitialState( initialState ) {
Expand All @@ -51,7 +51,10 @@ function loadInitialState( initialState ) {
debug( 'stored state is too old, building redux store from scratch' );
initialState = {};
}
return createReduxStore( deserialize( initialState ) );
const localforageState = deserialize( initialState );
const serverState = getInitialServerState();
const mergedState = Object.assign( {}, localforageState, serverState );
return createReduxStore( mergedState );
}

function loadInitialStateFailed( error ) {
Expand Down