Framework: add separate server state hydration #3343
Conversation
gwwar
added
the
[Status] Needs Review
| @@ -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 ) ); | |||
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
gwwar
force-pushed
the
try/hydrate-server-state
branch
from
60e80be to
1f9e867
Compare
| @@ -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'; | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
|
This is working fine for the server state currently in usage. Good to merge 👍 |
seear
added
[Status] Ready to Merge
and removed
[Status] Needs Review
gwwar
force-pushed
the
try/hydrate-server-state
branch
from
1f9e867 to
cf10763
Compare
Framework: add separate server state hydration
Following the conversation in #3337 This is an example of what a separate server state hydration pipeline might look like. Reducers would only need to implement this for subtrees that do not contain plain JavaScript objects. Feel free to call out other suggestions for the action name.
cc @aduth @seear