From bfd779b662f881b28d9cb94f55d29f8692113637 Mon Sep 17 00:00:00 2001 From: Bo Borgerson Date: Tue, 29 Mar 2016 16:26:22 -0700 Subject: [PATCH 01/12] Spit out an RSA cache dump with a trigger param --- .../react-server/core/renderMiddleware.js | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/packages/react-server/core/renderMiddleware.js b/packages/react-server/core/renderMiddleware.js index d54526db6..c164ecfff 100644 --- a/packages/react-server/core/renderMiddleware.js +++ b/packages/react-server/core/renderMiddleware.js @@ -205,6 +205,8 @@ function renderPage(req, res, context, start, page) { lifecycleMethods = fragmentLifecycle(); } else if (PageUtil.PageConfig.get('isRawResponse')){ lifecycleMethods = rawResponseLifecycle(); + } else if (req.query._react_server_preload_bundle) { + lifecycleMethods = preloadBundleLifecycle(); } else { lifecycleMethods = pageLifecycle(); } @@ -250,6 +252,16 @@ function fragmentLifecycle () { ]; } +function preloadBundleLifecycle () { + return [ + Q(), // NOOP lead-in to prime the reduction + setPreloadBundleContentType, + writePreloadBundle, + handleResponseComplete, + endResponse, + ]; +} + function pageLifecycle() { return [ Q(), // This is just a NOOP lead-in to prime the reduction. @@ -268,6 +280,10 @@ function setContentType(req, res, context, start, pageObject) { res.set('Content-Type', pageObject.getContentType()); } +function setPreloadBundleContentType(req, res) { + res.set('Content-Type', 'application/json'); +} + function writeHeader(req, res, context, start, pageObject) { res.type('html'); res.set('Transfer-Encoding', 'chunked'); @@ -736,6 +752,15 @@ function writeResponseData(req, res, context, start, page) { }); } +function writePreloadBundle(req, res) { + + const cache = ReactServerAgent.cache(); + + return Q.allSettled( + cache.getPendingRequests().map(v => v.entry.dfd.promise) + ).then(() => res.write(JSON.stringify(cache.dehydrate()))); +} + function renderElement(res, element, context) { if (element.containerOpen || element.containerClose){ From 3030f813179f411fc3b34e369e21818616d17a66 Mon Sep 17 00:00:00 2001 From: Bo Borgerson Date: Wed, 30 Mar 2016 15:52:18 -0700 Subject: [PATCH 02/12] Handle preload bundles --- packages/react-server/core/ClientController.js | 9 +++++++-- packages/react-server/core/ReactServerAgent.js | 13 +++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/packages/react-server/core/ClientController.js b/packages/react-server/core/ClientController.js index 0a1bb3ce0..bbaee6d65 100644 --- a/packages/react-server/core/ClientController.js +++ b/packages/react-server/core/ClientController.js @@ -88,13 +88,13 @@ class ClientController extends EventEmitter { _startRequest({request, type}) { + const url = request.getUrl(); + if (request.getFrameback()){ // Tell the navigator we got this one. this.context.navigator.ignoreCurrentNavigation(); - var url = request.getUrl(); - if (type === History.events.PUSHSTATE) { // Sorry folks. If we need to do a client // transition, then we're going to clobber @@ -135,6 +135,11 @@ class ClientController extends EventEmitter { RequestLocalStorage.startRequest(); + // If we've got a preload bundle let's inflate + // it and avoid firing off a bunch of xhr + // requests during `handleRoute`. + ReactServerAgent._checkForPreload(url); + // we need to re-register the request context // as a RequestLocal. this.context.registerRequestLocal(); diff --git a/packages/react-server/core/ReactServerAgent.js b/packages/react-server/core/ReactServerAgent.js index bbc6267d7..e1e9bfb35 100644 --- a/packages/react-server/core/ReactServerAgent.js +++ b/packages/react-server/core/ReactServerAgent.js @@ -10,6 +10,8 @@ function makeRequest (method, url) { return new Request(method, url, API.cache()); } +const PRELOADS = {}; + var API = { get (url, data) { @@ -97,6 +99,17 @@ var API = { Plugins.forResponse().add(pluginFunc); }, + preloadDataForURL (url) { + return this.get(url, {_react_server_preload_bundle: 1}) + .then(data => PRELOADS[url] = data.body); // eslint-disable-line no-return-assign + }, + + _checkForPreload (url) { + if (PRELOADS[url]) { + API.cache().rehydrate(PRELOADS[url]); + } + }, + } From 5ed34434efdc7acd6b5d380229b0858c83eaabeb Mon Sep 17 00:00:00 2001 From: Bo Borgerson Date: Thu, 31 Mar 2016 09:17:02 -0700 Subject: [PATCH 03/12] Don't re-download bundles that are already on hand --- packages/react-server/core/ReactServerAgent.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/react-server/core/ReactServerAgent.js b/packages/react-server/core/ReactServerAgent.js index e1e9bfb35..5caac1611 100644 --- a/packages/react-server/core/ReactServerAgent.js +++ b/packages/react-server/core/ReactServerAgent.js @@ -100,6 +100,7 @@ var API = { }, preloadDataForURL (url) { + if (PRELOADS[url]) return PRELOADS[url]; return this.get(url, {_react_server_preload_bundle: 1}) .then(data => PRELOADS[url] = data.body); // eslint-disable-line no-return-assign }, From 29e94e41550975213e5b30f20cb0d0cd6e7d1a3d Mon Sep 17 00:00:00 2001 From: Bo Borgerson Date: Thu, 31 Mar 2016 09:52:45 -0700 Subject: [PATCH 04/12] Only one preload request per URL --- .../react-server/core/ClientController.js | 5 ---- .../react-server/core/ReactServerAgent.js | 27 +++++++++++++------ .../react-server/core/context/Navigator.js | 10 ++++++- 3 files changed, 28 insertions(+), 14 deletions(-) diff --git a/packages/react-server/core/ClientController.js b/packages/react-server/core/ClientController.js index bbaee6d65..4582eb322 100644 --- a/packages/react-server/core/ClientController.js +++ b/packages/react-server/core/ClientController.js @@ -135,11 +135,6 @@ class ClientController extends EventEmitter { RequestLocalStorage.startRequest(); - // If we've got a preload bundle let's inflate - // it and avoid firing off a bunch of xhr - // requests during `handleRoute`. - ReactServerAgent._checkForPreload(url); - // we need to re-register the request context // as a RequestLocal. this.context.registerRequestLocal(); diff --git a/packages/react-server/core/ReactServerAgent.js b/packages/react-server/core/ReactServerAgent.js index 5caac1611..d87cf8303 100644 --- a/packages/react-server/core/ReactServerAgent.js +++ b/packages/react-server/core/ReactServerAgent.js @@ -1,4 +1,5 @@ var RLS = require('./util/RequestLocalStorage').getNamespace() +, Q = require("q") , Cache = require("./ReactServerAgent/Cache") , Request = require("./ReactServerAgent/Request") , Plugins = require("./ReactServerAgent/Plugins") @@ -10,10 +11,14 @@ function makeRequest (method, url) { return new Request(method, url, API.cache()); } -const PRELOADS = {}; +const BUNDLE_CACHE = {}; +const BUNDLE_PARAMETER = '_react_server_preload_bundle'; +const BUNDLE_OPTS = {[BUNDLE_PARAMETER]: 1}; var API = { + BUNDLE_PARAMETER, + get (url, data) { var req = makeRequest('GET', url); if (data) req.query(data); @@ -100,15 +105,21 @@ var API = { }, preloadDataForURL (url) { - if (PRELOADS[url]) return PRELOADS[url]; - return this.get(url, {_react_server_preload_bundle: 1}) - .then(data => PRELOADS[url] = data.body); // eslint-disable-line no-return-assign - }, + if (SERVER_SIDE) throw new Error("Can't preload server-side"); + if (!BUNDLE_CACHE[url]){ + BUNDLE_CACHE[url] = this.get(url, BUNDLE_OPTS) + .then(data => data.body); - _checkForPreload (url) { - if (PRELOADS[url]) { - API.cache().rehydrate(PRELOADS[url]); } + return BUNDLE_CACHE[url]; + }, + + _rehydratePreloadData(url) { + // If we don't have any then we can't use it. + if (!BUNDLE_CACHE[url]) return Q(); + + return BUNDLE_CACHE[url] + .then(data => API.cache().rehydrate(data)); }, } diff --git a/packages/react-server/core/context/Navigator.js b/packages/react-server/core/context/Navigator.js index a2277e59b..23a0b66c4 100644 --- a/packages/react-server/core/context/Navigator.js +++ b/packages/react-server/core/context/Navigator.js @@ -4,6 +4,7 @@ var EventEmitter = require('events').EventEmitter, Router = require('routr'), Q = require('q'), History = require("../components/History"), + ReactServerAgent = require("../ReactServerAgent"), PageUtil = require("../util/PageUtil"); var _ = { @@ -65,7 +66,14 @@ class Navigator extends EventEmitter { // The promise returned from `startRoute()` will be rejected // if we're not going to proceed, so resources will be freed. // - this.startRoute(route, request, type).then(() => { + this + .startRoute(route, request, type) + + // If we've got a preload bundle let's inflate it and avoid + // firing off a bunch of xhr requests during `handleRoute`. + .then(() => ReactServerAgent._rehydratePreloadData(request.getUrl())) + + .then(() => { if (this._ignoreCurrentNavigation){ // This is a one-time deal. this._ignoreCurrentNavigation = false; From 3f8352c2204f2d055304c57e22a48e1b537d58f2 Mon Sep 17 00:00:00 2001 From: Bo Borgerson Date: Thu, 31 Mar 2016 14:25:20 -0700 Subject: [PATCH 05/12] Pull out preload bundle fetcher --- packages/react-server/core/ReactServerAgent.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/react-server/core/ReactServerAgent.js b/packages/react-server/core/ReactServerAgent.js index d87cf8303..391eafcef 100644 --- a/packages/react-server/core/ReactServerAgent.js +++ b/packages/react-server/core/ReactServerAgent.js @@ -107,13 +107,15 @@ var API = { preloadDataForURL (url) { if (SERVER_SIDE) throw new Error("Can't preload server-side"); if (!BUNDLE_CACHE[url]){ - BUNDLE_CACHE[url] = this.get(url, BUNDLE_OPTS) - .then(data => data.body); - + BUNDLE_CACHE[url] = API._fetchPreloadData(url); } return BUNDLE_CACHE[url]; }, + _fetchPreloadData(url) { + return this.get(url, BUNDLE_OPTS).then(data => data.body); + }, + _rehydratePreloadData(url) { // If we don't have any then we can't use it. if (!BUNDLE_CACHE[url]) return Q(); From 43b0fa7b34debd48855b615731db216f47c96fca Mon Sep 17 00:00:00 2001 From: Jennifer Hwang Date: Thu, 31 Mar 2016 17:37:17 -0700 Subject: [PATCH 06/12] Parse + stringify data when getting it in/out of cache to create deep copies. Shallow copy was being modified by different client transitions? --- packages/react-server/core/ReactServerAgent.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react-server/core/ReactServerAgent.js b/packages/react-server/core/ReactServerAgent.js index 391eafcef..fe59bb1e3 100644 --- a/packages/react-server/core/ReactServerAgent.js +++ b/packages/react-server/core/ReactServerAgent.js @@ -113,7 +113,7 @@ var API = { }, _fetchPreloadData(url) { - return this.get(url, BUNDLE_OPTS).then(data => data.body); + return this.get(url, BUNDLE_OPTS).then(data => JSON.stringify(data.body)); }, _rehydratePreloadData(url) { @@ -121,7 +121,7 @@ var API = { if (!BUNDLE_CACHE[url]) return Q(); return BUNDLE_CACHE[url] - .then(data => API.cache().rehydrate(data)); + .then(data => API.cache().rehydrate(JSON.parse(data))); }, } From e5270f7deae4ffc12c8c5dd97353b82e6f9446cb Mon Sep 17 00:00:00 2001 From: Bo Borgerson Date: Tue, 5 Apr 2016 13:18:17 -0700 Subject: [PATCH 07/12] Put `url` back where we found it (PR diff cleanup) --- packages/react-server/core/ClientController.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react-server/core/ClientController.js b/packages/react-server/core/ClientController.js index 4582eb322..0a1bb3ce0 100644 --- a/packages/react-server/core/ClientController.js +++ b/packages/react-server/core/ClientController.js @@ -88,13 +88,13 @@ class ClientController extends EventEmitter { _startRequest({request, type}) { - const url = request.getUrl(); - if (request.getFrameback()){ // Tell the navigator we got this one. this.context.navigator.ignoreCurrentNavigation(); + var url = request.getUrl(); + if (type === History.events.PUSHSTATE) { // Sorry folks. If we need to do a client // transition, then we're going to clobber From 24b6a5f407e5199c529b861afadb4cbba87b3d29 Mon Sep 17 00:00:00 2001 From: Bo Borgerson Date: Tue, 5 Apr 2016 15:24:07 -0700 Subject: [PATCH 08/12] Better and more consistent names for preload data bundles Just "bundle" is ambiguous since we also use webpack JS bundles. --- .../react-server/core/ReactServerAgent.js | 24 +++++++++---------- .../react-server/core/context/Navigator.js | 2 +- .../react-server/core/renderMiddleware.js | 8 +++---- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/packages/react-server/core/ReactServerAgent.js b/packages/react-server/core/ReactServerAgent.js index fe59bb1e3..f7cfd40af 100644 --- a/packages/react-server/core/ReactServerAgent.js +++ b/packages/react-server/core/ReactServerAgent.js @@ -11,13 +11,13 @@ function makeRequest (method, url) { return new Request(method, url, API.cache()); } -const BUNDLE_CACHE = {}; -const BUNDLE_PARAMETER = '_react_server_preload_bundle'; -const BUNDLE_OPTS = {[BUNDLE_PARAMETER]: 1}; +const DATA_BUNDLE_CACHE = {}; +const DATA_BUNDLE_PARAMETER = '_react_server_data_bundle'; +const DATA_BUNDLE_OPTS = {[DATA_BUNDLE_PARAMETER]: 1}; var API = { - BUNDLE_PARAMETER, + DATA_BUNDLE_PARAMETER, get (url, data) { var req = makeRequest('GET', url); @@ -106,21 +106,21 @@ var API = { preloadDataForURL (url) { if (SERVER_SIDE) throw new Error("Can't preload server-side"); - if (!BUNDLE_CACHE[url]){ - BUNDLE_CACHE[url] = API._fetchPreloadData(url); + if (!DATA_BUNDLE_CACHE[url]){ + DATA_BUNDLE_CACHE[url] = API._fetchDataBundle(url); } - return BUNDLE_CACHE[url]; + return DATA_BUNDLE_CACHE[url]; }, - _fetchPreloadData(url) { - return this.get(url, BUNDLE_OPTS).then(data => JSON.stringify(data.body)); + _fetchDataBundle(url) { + return this.get(url, DATA_BUNDLE_OPTS).then(data => JSON.stringify(data.body)); }, - _rehydratePreloadData(url) { + _rehydrateDataBundle(url) { // If we don't have any then we can't use it. - if (!BUNDLE_CACHE[url]) return Q(); + if (!DATA_BUNDLE_CACHE[url]) return Q(); - return BUNDLE_CACHE[url] + return DATA_BUNDLE_CACHE[url] .then(data => API.cache().rehydrate(JSON.parse(data))); }, diff --git a/packages/react-server/core/context/Navigator.js b/packages/react-server/core/context/Navigator.js index 23a0b66c4..83af0e2a3 100644 --- a/packages/react-server/core/context/Navigator.js +++ b/packages/react-server/core/context/Navigator.js @@ -71,7 +71,7 @@ class Navigator extends EventEmitter { // If we've got a preload bundle let's inflate it and avoid // firing off a bunch of xhr requests during `handleRoute`. - .then(() => ReactServerAgent._rehydratePreloadData(request.getUrl())) + .then(() => ReactServerAgent._rehydrateDataBundle(request.getUrl())) .then(() => { if (this._ignoreCurrentNavigation){ diff --git a/packages/react-server/core/renderMiddleware.js b/packages/react-server/core/renderMiddleware.js index c164ecfff..a5649d20e 100644 --- a/packages/react-server/core/renderMiddleware.js +++ b/packages/react-server/core/renderMiddleware.js @@ -255,8 +255,8 @@ function fragmentLifecycle () { function preloadBundleLifecycle () { return [ Q(), // NOOP lead-in to prime the reduction - setPreloadBundleContentType, - writePreloadBundle, + setDataBundleContentType, + writeDataBundle, handleResponseComplete, endResponse, ]; @@ -280,7 +280,7 @@ function setContentType(req, res, context, start, pageObject) { res.set('Content-Type', pageObject.getContentType()); } -function setPreloadBundleContentType(req, res) { +function setDataBundleContentType(req, res) { res.set('Content-Type', 'application/json'); } @@ -752,7 +752,7 @@ function writeResponseData(req, res, context, start, page) { }); } -function writePreloadBundle(req, res) { +function writeDataBundle(req, res) { const cache = ReactServerAgent.cache(); From 82ea532b86c26bb8d7a7ca648fc2052c8f23883a Mon Sep 17 00:00:00 2001 From: Bo Borgerson Date: Tue, 5 Apr 2016 15:26:16 -0700 Subject: [PATCH 09/12] Use exported parameter name constant Eliminate a duplicate magic string --- packages/react-server/core/renderMiddleware.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-server/core/renderMiddleware.js b/packages/react-server/core/renderMiddleware.js index a5649d20e..fcad34012 100644 --- a/packages/react-server/core/renderMiddleware.js +++ b/packages/react-server/core/renderMiddleware.js @@ -205,7 +205,7 @@ function renderPage(req, res, context, start, page) { lifecycleMethods = fragmentLifecycle(); } else if (PageUtil.PageConfig.get('isRawResponse')){ lifecycleMethods = rawResponseLifecycle(); - } else if (req.query._react_server_preload_bundle) { + } else if (req.query[ReactServerAgent.DATA_BUNDLE_PARAMETER]) { lifecycleMethods = preloadBundleLifecycle(); } else { lifecycleMethods = pageLifecycle(); From b6e9eee91c88acf6a35c2aca73776e8d54b0acc7 Mon Sep 17 00:00:00 2001 From: Bo Borgerson Date: Tue, 5 Apr 2016 15:27:03 -0700 Subject: [PATCH 10/12] Make another function name consistent --- packages/react-server/core/renderMiddleware.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react-server/core/renderMiddleware.js b/packages/react-server/core/renderMiddleware.js index fcad34012..fba63d141 100644 --- a/packages/react-server/core/renderMiddleware.js +++ b/packages/react-server/core/renderMiddleware.js @@ -206,7 +206,7 @@ function renderPage(req, res, context, start, page) { } else if (PageUtil.PageConfig.get('isRawResponse')){ lifecycleMethods = rawResponseLifecycle(); } else if (req.query[ReactServerAgent.DATA_BUNDLE_PARAMETER]) { - lifecycleMethods = preloadBundleLifecycle(); + lifecycleMethods = dataBundleLifecycle(); } else { lifecycleMethods = pageLifecycle(); } @@ -252,7 +252,7 @@ function fragmentLifecycle () { ]; } -function preloadBundleLifecycle () { +function dataBundleLifecycle () { return [ Q(), // NOOP lead-in to prime the reduction setDataBundleContentType, From 3a195a4fbb6d3d3bc8e385caf76c8f09fb69c3a0 Mon Sep 17 00:00:00 2001 From: Bo Borgerson Date: Wed, 6 Apr 2016 11:43:04 -0700 Subject: [PATCH 11/12] Add comment about and guard against server-side cache usage Want to be extra sure that we don't accidentally cache data across requests on the server. --- packages/react-server/core/ReactServerAgent.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/react-server/core/ReactServerAgent.js b/packages/react-server/core/ReactServerAgent.js index f7cfd40af..82801532d 100644 --- a/packages/react-server/core/ReactServerAgent.js +++ b/packages/react-server/core/ReactServerAgent.js @@ -11,7 +11,10 @@ function makeRequest (method, url) { return new Request(method, url, API.cache()); } -const DATA_BUNDLE_CACHE = {}; +// REALLY don't want to accidentally cache data across requests on the server. +// We throw an error if `preloadDataForURL` is called server-side, but it's +// worth being doubly cautious here. +const DATA_BUNDLE_CACHE = SERVER_SIDE?undefined:{}; const DATA_BUNDLE_PARAMETER = '_react_server_data_bundle'; const DATA_BUNDLE_OPTS = {[DATA_BUNDLE_PARAMETER]: 1}; From 1e2b034d9a243428147ba2ac6410216fbc4191e3 Mon Sep 17 00:00:00 2001 From: Bo Borgerson Date: Wed, 6 Apr 2016 12:02:58 -0700 Subject: [PATCH 12/12] Use a frozen object instead of undefined for cache server-side We access it in a few places, but we don't want to be able to _write_ to it. --- packages/react-server/core/ReactServerAgent.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-server/core/ReactServerAgent.js b/packages/react-server/core/ReactServerAgent.js index 82801532d..b9d64e06f 100644 --- a/packages/react-server/core/ReactServerAgent.js +++ b/packages/react-server/core/ReactServerAgent.js @@ -14,7 +14,7 @@ function makeRequest (method, url) { // REALLY don't want to accidentally cache data across requests on the server. // We throw an error if `preloadDataForURL` is called server-side, but it's // worth being doubly cautious here. -const DATA_BUNDLE_CACHE = SERVER_SIDE?undefined:{}; +const DATA_BUNDLE_CACHE = SERVER_SIDE?Object.freeze({}):{}; const DATA_BUNDLE_PARAMETER = '_react_server_data_bundle'; const DATA_BUNDLE_OPTS = {[DATA_BUNDLE_PARAMETER]: 1};