-
Notifications
You must be signed in to change notification settings - Fork 19
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.
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
Add automatic SSR data hydration support #22
base: master
Are you sure you want to change the base?
Changes from 2 commits
0e89dee
4ad37cd
72dbfa5
203a104
5f00bef
5902caa
26efcf6
0aa5744
48df697
81c34c6
c0389b1
ba57e5e
ef65341
6e6fce8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
import { Promise } from 'meteor/promise' | ||
import { generateQueryId } from './utils.js'; | ||
|
||
export default { | ||
|
||
decodeData(data) { | ||
const decodedEjsonString = decodeURIComponent(data); | ||
if (!decodedEjsonString) return null; | ||
|
||
return EJSON.parse(decodedEjsonString); | ||
}, | ||
|
||
load(optns) { | ||
const defaults = { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. specify defaults outside the function itself There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, why 3s for self-destructing ? Isn't that unsafe ? |
||
selfDestruct: 3000 | ||
} | ||
const options = Object.assign({}, defaults, optns) | ||
|
||
return new Promise((resolve, reject) => { | ||
// Retrieve the payload from the DOM | ||
const dom = document.querySelectorAll( | ||
'script[type="text/grapher-data"]', | ||
document | ||
); | ||
const dataString = dom && dom.length > 0 ? dom[0].innerHTML : ''; | ||
const data = this.decodeData(dataString) || {}; | ||
window.grapherQueryStore = data | ||
|
||
// Self destruct the store so that dynamically loaded modules | ||
// do not pull from the store in the future | ||
setTimeout(() => { | ||
window.grapherQueryStore = {}; | ||
}, options.selfDestruct) | ||
|
||
resolve(data); | ||
}); | ||
}, | ||
|
||
getQueryData(query) { | ||
const id = generateQueryId(query); | ||
const data = window.grapherQueryStore[id] | ||
return data | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
import React from 'react' | ||
import PropTypes from 'prop-types'; | ||
import { EJSON } from 'meteor/ejson'; | ||
import { generateQueryId } from './utils.js'; | ||
export const SSRDataStoreContext = React.createContext(null); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This release is going to break React 15.x, we need to make it optional. We still want to allow latest versions of grapher-react to be used with 15.x. |
||
|
||
class DataStore { | ||
storage = {} | ||
|
||
add(query, value) { | ||
const key = generateQueryId(query) | ||
this.storage[key] = value | ||
} | ||
|
||
getData() { | ||
return this.storage | ||
} | ||
} | ||
|
||
export default class SSRDataStore{ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. use consistent Coding standards (space before {) |
||
constructor() { | ||
this.store = new DataStore() | ||
} | ||
|
||
collectData(children) { | ||
return <SSRDataStoreContext.Provider value={this.store}>{children}</SSRDataStoreContext.Provider> | ||
} | ||
|
||
encodeData(data) { | ||
data = EJSON.stringify(data) | ||
return encodeURIComponent(data) | ||
} | ||
|
||
getScriptTags() { | ||
const data = this.store.getData() | ||
|
||
return `<script type="text/grapher-data">${this.encodeData(data)}</script>` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. use text/grapher-data, as a constant variable somewhere. Let's not rely on strings for things that are re-used in multiple places. |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
export const generateQueryId = function (query) { | ||
return `${query.queryName}::${EJSON.stringify(query.params)}`; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,7 @@ | ||
import {withTracker} from 'meteor/react-meteor-data'; | ||
import {ReactiveVar} from 'meteor/reactive-var'; | ||
import DataHydrator from './DataHydrator.js'; | ||
import { Meteor } from 'meteor/meteor'; | ||
|
||
/** | ||
* Wraps the query and provides reactive data fetching utility | ||
|
@@ -14,24 +16,48 @@ export default function withReactiveContainer(handler, config, QueryComponent) { | |
return withTracker((props) => { | ||
const query = handler(props); | ||
|
||
const subscriptionHandle = query.subscribe({ | ||
onStop(err) { | ||
if (err) { | ||
subscriptionError.set(err); | ||
} | ||
}, | ||
onReady() { | ||
subscriptionError.set(null); | ||
} | ||
}); | ||
let isLoading | ||
let data | ||
let error | ||
|
||
const isReady = subscriptionHandle.ready(); | ||
// For server-side-rendering, immediately fetch the data | ||
// and save it in the data store for this request | ||
|
||
if(Meteor.isServer) { | ||
data = query.fetch(); | ||
isLoading = false; | ||
props.dataStore.add(query, data); | ||
} else { | ||
const subscriptionHandle = query.subscribe({ | ||
onStop(err) { | ||
if (err) { | ||
subscriptionError.set(err); | ||
} | ||
}, | ||
onReady() { | ||
subscriptionError.set(null); | ||
} | ||
}); | ||
|
||
isLoading = !subscriptionHandle.ready(); | ||
|
||
const data = query.fetch(); | ||
// Check the SSR query store for data | ||
if(Meteor.isClient && window && window.grapherQueryStore) { | ||
const ssrData = DataHydrator.getQueryData(query); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I now understand why you are doing the self-destruct thingie. But a healthier approach would be to set a flag once first hydration has been made, and do the check in here. I don't know something like And pls use consistent styles |
||
if(ssrData) { | ||
isLoading = false; | ||
data = ssrData; | ||
} | ||
} | ||
|
||
if(!data) { | ||
data = query.fetch(); | ||
} | ||
} | ||
|
||
return { | ||
grapher: { | ||
isLoading: !isReady, | ||
isLoading, | ||
data, | ||
error: subscriptionError, | ||
}, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,17 +1,48 @@ | ||
import React from 'react'; | ||
import getDisplayName from './getDisplayName'; | ||
import { Meteor } from 'meteor/meteor'; | ||
import DataHydrator from './DataHydrator.js'; | ||
|
||
export default function withStaticQueryContainer(WrappedComponent) { | ||
/** | ||
* We use it like this so we can have naming inside React Dev Tools | ||
* This is a standard pattern in HOCs | ||
*/ | ||
class GrapherStaticQueryContainer extends React.Component { | ||
state = { | ||
isLoading: true, | ||
error: null, | ||
data: [], | ||
}; | ||
constructor(props) { | ||
super(props); | ||
|
||
this.state = { | ||
isLoading: true, | ||
error: null, | ||
data: [], | ||
}; | ||
|
||
const { query } = props; | ||
|
||
// Check the SSR query store for data | ||
if(Meteor.isClient && window && window.grapherQueryStore) { | ||
const data = DataHydrator.getQueryData(query); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. create 2 additional methods for this, there's too much code coupled in the constructor. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what if the query gets an error ? how is this treated ? |
||
if(data) { | ||
this.state = { | ||
isLoading: false, | ||
data | ||
}; | ||
|
||
} | ||
} | ||
|
||
// For server-side-rendering, immediately fetch the data | ||
// and save it in the data store for this request | ||
if(Meteor.isServer) { | ||
const data = query.fetch(); | ||
this.state = { | ||
isLoading: false, | ||
data | ||
}; | ||
props.dataStore.add(query, data); | ||
} | ||
} | ||
|
||
componentWillReceiveProps(nextProps) { | ||
const {query} = nextProps; | ||
|
@@ -20,7 +51,11 @@ export default function withStaticQueryContainer(WrappedComponent) { | |
|
||
componentDidMount() { | ||
const {query, config} = this.props; | ||
this.fetch(query); | ||
|
||
// Do not fetch is we already have the data from SSR hydration | ||
if(this.state.isLoading === true) { | ||
this.fetch(query); | ||
} | ||
|
||
if (config.pollingMs) { | ||
this.pollingInterval = setInterval(() => { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ import withReactiveQuery from './lib/withReactiveQuery'; | |
import withQueryContainer from './lib/withQueryContainer'; | ||
import withStaticQuery from './lib/withStaticQuery'; | ||
import checkOptions from './lib/checkOptions'; | ||
import { SSRDataStoreContext } from './lib/SSRDataStore.js' | ||
|
||
export default function (handler, _config = {}) { | ||
checkOptions(_config); | ||
|
@@ -14,19 +15,26 @@ export default function (handler, _config = {}) { | |
const queryContainer = withQueryContainer(component); | ||
|
||
if (!config.reactive) { | ||
const staticQueryContainer = withStaticQuery(queryContainer); | ||
const StaticQueryContainer = withStaticQuery(queryContainer); | ||
|
||
return function (props) { | ||
const query = handler(props); | ||
|
||
return React.createElement(staticQueryContainer, { | ||
query, | ||
props, | ||
config | ||
}) | ||
return ( | ||
<SSRDataStoreContext.Consumer> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I fail to understand why you need context, if you can use window.GRAPHER = { queryStore: {}, hydrationCompleted: bool, dataStore } |
||
{dataStore => <StaticQueryContainer query={query} props={props} config={config} dataStore={dataStore} />} | ||
</SSRDataStoreContext.Consumer> | ||
) | ||
} | ||
} else { | ||
return withReactiveQuery(handler, config, queryContainer); | ||
const ReactiveQueryContainer = withReactiveQuery(handler, config, queryContainer); | ||
return function(props) { | ||
return ( | ||
<SSRDataStoreContext.Consumer> | ||
{dataStore => <ReactiveQueryContainer dataStore={dataStore} />} | ||
</SSRDataStoreContext.Consumer> | ||
) | ||
} | ||
} | ||
}; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
propper 4 space indentation please