From 971b8ced560f93e3044a0ee06baff9fe4cb70fa9 Mon Sep 17 00:00:00 2001
From: Bo Borgerson <gigabo@gmail.com>
Date: Wed, 25 May 2016 17:12:21 -0700
Subject: [PATCH 1/2] Add new component: `<TheFold />`

This replaces the `getAboveTheFoldCount()` page method, which is awful:

- It's separate from the elements themselves, so even remembering to update it
  can be a challenge.
- If the number of root elements above the fold varies, then duplicative logic
  may be required to determine the count.
- Middleware that inserts elements must remember to _increment_ it.
- It has non-obvious behavior when root containers are used above the fold (#144)

This closes #161 where @bartkusa proposed this _much_ nicer interface.

This is a breaking change.  The _default_ was previously to wake up the client
controller after the _first_ element.  With this change the client controller
isn't bootstrapped until either `<TheFold />` is seen, or we're out of
elements.  So, if unspecified, the entire page is assumed to be above the
fold.
---
 docs/understanding-rendering.md               |  7 ++-
 docs/writing-pages.md                         |  1 -
 .../pages/data/delay.js                       |  7 ++-
 .../pages/root/aboveTheFold.js                | 22 ++++-----
 .../react-server/core/ClientController.js     | 18 +++----
 packages/react-server/core/common.js          |  1 +
 .../core/components/RootElement.js            |  6 ++-
 .../react-server/core/components/TheFold.js   | 19 ++++++++
 .../react-server/core/renderMiddleware.js     | 47 ++++++++++++-------
 packages/react-server/core/util/PageUtil.js   |  3 +-
 10 files changed, 83 insertions(+), 48 deletions(-)
 create mode 100644 packages/react-server/core/components/TheFold.js

diff --git a/docs/understanding-rendering.md b/docs/understanding-rendering.md
index ae13b7b4b..ff4fa8bbb 100644
--- a/docs/understanding-rendering.md
+++ b/docs/understanding-rendering.md
@@ -28,6 +28,7 @@ getElements() {
 				<Sidebar  />
 			</RootElement>
 		</RootContainer>
+		<TheFold />
 		<Footer />
 	</RootContainer>
 }
@@ -44,6 +45,10 @@ In the example above:
   the all objects from `bodyEmitter` and the resolution of `sidebarPromise`.
   Client-side it will re-render with updated props from `bodyEmitter` if it
   fires after the initial render.
+- `<TheFold />` will cause an inline `<script>` tag to be sent that kicks off
+  the client-side render, making elements above the fold interactive.  This
+  forces a browser paint, so it's important to put it after above-the-fold
+  elements.
 - `<Footer />` Will render immediately, and will be sent to the browser as
   soon as all elements before it have rendered and been sent.  It won't
   receive any props.
@@ -55,7 +60,7 @@ In the example above:
 - If an element is blocking already-rendered elements after it, when it
   renders the entire block of elements will be sent in a single `write` to the
   response socket.
-- After the above-the-fold elements (as specified by `getAboveTheFoldCount()`)
+- After the above-the-fold elements (followed by `<TheFold/>`)
   have been sent an inline `<script>` is sent that instantiates the
   `ClientController` in the browser and gives it the data bundle for all
   requests that have resolved.  It then renders all elements that have already
diff --git a/docs/writing-pages.md b/docs/writing-pages.md
index 8279e0e77..62b7fb6c9 100644
--- a/docs/writing-pages.md
+++ b/docs/writing-pages.md
@@ -45,7 +45,6 @@ lifecycle is called in order
 1. getBodyClasses
 1. getBodyStartContent
 1. getElements
-1. getAboveTheFoldCount (until we've rendered all the above-the-fold content)
 
 Once we reach the above the fold content, we'll start sending javascript.
 
diff --git a/packages/react-server-test-pages/pages/data/delay.js b/packages/react-server-test-pages/pages/data/delay.js
index a2ee5247e..2c2a53d27 100644
--- a/packages/react-server-test-pages/pages/data/delay.js
+++ b/packages/react-server-test-pages/pages/data/delay.js
@@ -1,4 +1,7 @@
 import Q from "q";
+import _ from "lodash";
+
+const BIG = n => _.range(+n).reduce((m, v) => (m['element_'+v] = v, m), {})
 
 export default class DelayDataPage {
 	setConfigValues() {
@@ -8,8 +11,8 @@ export default class DelayDataPage {
 		return 'application/json';
 	}
 	getResponseData() {
-		const { ms, val } = this.getRequest().getQuery();
+		const { ms, val, big } = this.getRequest().getQuery();
 		return Q.delay(ms||0)
-			.then(() => val||JSON.stringify({'ok':true}));
+			.then(() => val||JSON.stringify(big?BIG(big):{'ok':true}));
 	}
 }
diff --git a/packages/react-server-test-pages/pages/root/aboveTheFold.js b/packages/react-server-test-pages/pages/root/aboveTheFold.js
index 70cf20e7e..b63d7f64e 100644
--- a/packages/react-server-test-pages/pages/root/aboveTheFold.js
+++ b/packages/react-server-test-pages/pages/root/aboveTheFold.js
@@ -1,28 +1,28 @@
-import {ReactServerAgent, RootContainer, RootElement} from "react-server"; // eslint-disable-line
-
-// TODO: when we implement <TheFold/> (https://github.com/redfin/react-server/issues/161),
-// update this test page to use the new API
+import {
+	ReactServerAgent,
+	RootContainer,
+	RootElement,
+	TheFold,
+} from "react-server";
 
 export default class RootWhenPage {
 	handleRoute(next) {
-		this.data = ReactServerAgent.get('/data/delay?ms=200');
+		this.data = ReactServerAgent.get('/data/delay?ms=200&big=10000')
+			.then(res => res.body);
 		return next();
 	}
 	getElements() {
 		return [
-			<RootContainer>
+			<RootContainer when={this.data}>
 				<div>One</div>
 			</RootContainer>,
 			<RootElement when={this.data}><div>Two</div></RootElement>,
 			<RootContainer>
-				<div>Three - there should be script tags starting from right after me b/c my getAboveTheFoldCount is 3!</div>
+				<div>Three - there should be script tags starting from right after me.</div>
+				<TheFold />
 				<RootElement when={this.data}><div>Four</div></RootElement>
 			</RootContainer>,
 			<div>Five</div>,
 		]
 	}
-
-	getAboveTheFoldCount() {
-		return 3;
-	}
 }
diff --git a/packages/react-server/core/ClientController.js b/packages/react-server/core/ClientController.js
index 8f2a42723..d306dda67 100644
--- a/packages/react-server/core/ClientController.js
+++ b/packages/react-server/core/ClientController.js
@@ -497,11 +497,6 @@ class ClientController extends EventEmitter {
 		// each request so we can keep track of that overhead.
 		var totalRenderTime = 0;
 
-		// We'll consider the page "interactive" when we've rendered
-		// all elements that we expect to be above the fold.
-		// Need this to be an integer value greater than zero.
-		var aboveTheFoldCount = Math.max(page.getAboveTheFoldCount()|0, 1)
-
 		// These resolve with React elements when their data
 		// dependencies are fulfilled.
 		var elementPromises = PageUtil.standardizeElements(page.getElements());
@@ -601,7 +596,7 @@ class ClientController extends EventEmitter {
 						// If we're closing a container its
 						// parent is once again our mountNode.
 						mountNode = mountNode.parentNode;
-					} else {
+					} else if (!element.isTheFold) {
 
 						// Need a new root element in our
 						// current mountNode.
@@ -613,6 +608,11 @@ class ClientController extends EventEmitter {
 
 			if (element.containerOpen || element.containerClose){
 				return; // Nothing left to do.
+			} else if (element.isTheFold) {
+				logger.time(`renderAboveTheFold.fromStart`, new Date - tStart);
+				logger.time(`renderAboveTheFold.individual`, totalRenderTime);
+				logger.time(`renderAboveTheFold.elementCount`, index + 1);
+				return; // Again, this isn't a real root element.
 			}
 
 			var name  = PageUtil.getElementDisplayName(element)
@@ -632,12 +632,6 @@ class ClientController extends EventEmitter {
 				var tDisplay = root.getAttribute('data-react-server-timing-offset');
 				logger.time(`displayElement.fromStart.${name}`, +tDisplay);
 				logger.time(`renderElement.fromStart.${name}`, new Date - tStart);
-
-				if (index === aboveTheFoldCount - 1) {
-					logger.time(`renderAboveTheFold.fromStart`, new Date - tStart);
-					logger.time(`renderAboveTheFold.individual`, totalRenderTime);
-					logger.time(`renderAboveTheFold.elementCount`, aboveTheFoldCount);
-				}
 			}
 		};
 
diff --git a/packages/react-server/core/common.js b/packages/react-server/core/common.js
index ab7b473bd..276cd4d60 100644
--- a/packages/react-server/core/common.js
+++ b/packages/react-server/core/common.js
@@ -4,6 +4,7 @@ module.exports = {
 	RootContainer: require("./components/RootContainer"),
 	RootElement: require("./components/RootElement"),
 	Link: require('./components/Link'),
+	TheFold: require('./components/TheFold').default,
 	History: require('./components/History'),
 	ClientRequest: require('./ClientRequest'),
 	FramebackController: require('./FramebackController'),
diff --git a/packages/react-server/core/components/RootElement.js b/packages/react-server/core/components/RootElement.js
index fd9ea43e7..7c8f21c71 100644
--- a/packages/react-server/core/components/RootElement.js
+++ b/packages/react-server/core/components/RootElement.js
@@ -1,6 +1,8 @@
 var React = require('react');
 var Q = require('q');
 
+const {isTheFold} = require('./TheFold');
+
 const _ = {
 	assign: require('lodash/assign'),
 };
@@ -107,8 +109,8 @@ RootElement.getRootElementAttributes = function(element) {
 
 RootElement.ensureRootElementWithContainer = function(element, container) {
 
-	// If it's _already_ a root element, pass it along.
-	if (RootElement.isRootElement(element) || (
+	// If it's _already_ a root element (or the fold), pass it along.
+	if (RootElement.isRootElement(element) || isTheFold(element) || (
 
 		// Alternatively, if it's a control object pass it along.
 		//
diff --git a/packages/react-server/core/components/TheFold.js b/packages/react-server/core/components/TheFold.js
new file mode 100644
index 000000000..a6202cdf1
--- /dev/null
+++ b/packages/react-server/core/components/TheFold.js
@@ -0,0 +1,19 @@
+import {Component} from "react";
+
+export default class TheFold extends Component {
+	render() {
+		throw new Error("Something went wrong.  Trying to render the fold...");
+	}
+}
+
+TheFold.defaultProps = {
+	_isTheFold: true,
+}
+
+export function isTheFold(element) {
+	return element && element.props && element.props._isTheFold;
+}
+
+export function markTheFold() {
+	return {isTheFold:true};
+}
diff --git a/packages/react-server/core/renderMiddleware.js b/packages/react-server/core/renderMiddleware.js
index 27b27f174..6086ed689 100644
--- a/packages/react-server/core/renderMiddleware.js
+++ b/packages/react-server/core/renderMiddleware.js
@@ -633,10 +633,6 @@ function writeBody(req, res, context, start, page) {
 	// standardize to an array of EarlyPromises of ReactElements
 	var elementPromises = PageUtil.standardizeElements(page.getElements());
 
-	// No JS until the HTML above the fold has made it through.
-	// Need this to be an integer value greater than zero.
-	RLS().atfCount = Math.max(page.getAboveTheFoldCount()|0, 1);
-
 	// This is where we'll store our rendered HTML strings.  A value of
 	// `undefined` means we haven't rendered that element yet.
 	var rendered = elementPromises.map(() => ELEMENT_PENDING);
@@ -769,9 +765,9 @@ function writeDataBundle(req, res) {
 
 function renderElement(res, element, context) {
 
-	if (element.containerOpen || element.containerClose){
+	if (element.containerOpen || element.containerClose || element.isTheFold){
 
-		// Short-circuit out.  Don't want timing for containers.
+		// Short-circuit out.  Don't want timing for control objects.
 		return element;
 	}
 
@@ -832,19 +828,16 @@ function writeElements(res, elements) {
 
 		if (PageUtil.PageConfig.get('isFragment')) continue;
 
-		if (i === RLS().atfCount - 1){
+		if (RLS().haveBootstrapped) {
 
-			logAboveTheFoldTime(res);
-			// Okay, we've sent all of our above-the-fold HTML,
-			// now we can let the client start waking nodes up.
-			bootstrapClient(res)
-			for (var j = 0; j <= i; j++){
-				renderScriptsAsync([{ text: `__reactServerClientController.nodeArrival(${j})` }], res)
-			}
-		} else if (i >= RLS().atfCount){
+			// We've already bootstrapped, so we can immediately tell the
+			// client controller to wake the new element we just sent.
+			wakeElement(res, i);
+		} else if (i === elements.length - 1) {
 
-			// Let the client know it's there.
-			renderScriptsAsync([{ text: `__reactServerClientController.nodeArrival(${i})` }], res)
+			// Page didn't emit `<TheFold/>`.  Now we're done.
+			// This wakes everything up through `i`.
+			bootstrapClient(res, i);
 		}
 	}
 
@@ -869,6 +862,11 @@ function writeElement(res, element, i){
 		}>`);
 	} else if (element.containerClose) {
 		res.write('</div>');
+	} else if (element.isTheFold) {
+
+		// Okay, we've sent all of our above-the-fold HTML,
+		// now we can let the client start waking nodes up.
+		bootstrapClient(res, i)
 	} else {
 		res.write(`<div data-react-server-root-id=${
 			i
@@ -888,7 +886,10 @@ function logAboveTheFoldTime(res) {
 		'window.performance && window.performance.mark && window.performance.mark("displayAboveTheFold.fromStart");'}], res);
 }
 
-function bootstrapClient(res) {
+function bootstrapClient(res, lastElementSent) {
+
+	logAboveTheFoldTime(res);
+
 	var initialContext = {
 		'ReactServerAgent.cache': ReactServerAgent.cache().dehydrate(),
 	};
@@ -907,6 +908,16 @@ function bootstrapClient(res) {
 	// This actually needs to happen _synchronously_ with this current
 	// function to avoid letting responses slip in between.
 	setupLateArrivals(res);
+
+	for (var i = 0; i <= lastElementSent; i++) {
+		wakeElement(res, i);
+	}
+
+	RLS().haveBootstrapped = true;
+}
+
+function wakeElement(res, i) {
+	renderScriptsAsync([{ text: `__reactServerClientController.nodeArrival(${i})` }], res)
 }
 
 function setupLateArrivals(res) {
diff --git a/packages/react-server/core/util/PageUtil.js b/packages/react-server/core/util/PageUtil.js
index 5b751209a..30a4e1a25 100644
--- a/packages/react-server/core/util/PageUtil.js
+++ b/packages/react-server/core/util/PageUtil.js
@@ -5,6 +5,7 @@ var Q = require("q"),
 
 var {isRootContainer, flattenForRender} = require('../components/RootContainer');
 var {ensureRootElement, scheduleRender} = require('../components/RootElement');
+var {isTheFold, markTheFold} = require('../components/TheFold');
 
 // There are three data structures defined here that are relevant for page and
 // middleware authors:
@@ -61,7 +62,6 @@ var PAGE_METHODS = {
 	getBase            : [() => null, Q],
 	getBodyClasses     : [() => [], Q],
 	getElements        : [() => [], standardizeElements],
-	getAboveTheFoldCount : [() => 1, _ => _],
 	getResponseData    : [() => "", Q],
 };
 
@@ -174,6 +174,7 @@ function standardizeElements(elements) {
 		.makeArray(elements)
 		.map(e => isRootContainer(e)?flattenForRender(e):e)
 		.reduce((m, e) => m.concat(Array.isArray(e)?e:[e]), [])
+		.map(e => isTheFold(e)?markTheFold():e)
 		.map(ensureRootElement)
 		.map(scheduleRender)
 }

From 9c2bf19a6e89296e4956b9d1720a5392336e98f4 Mon Sep 17 00:00:00 2001
From: Bo Borgerson <gigabo@gmail.com>
Date: Thu, 26 May 2016 11:15:45 -0700
Subject: [PATCH 2/2] Don't log `renderAboveTheFold` times on client transition

Maybe we _should_, but we didn't previously.  This preserves existing behavior.
---
 packages/react-server/core/ClientController.js | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/packages/react-server/core/ClientController.js b/packages/react-server/core/ClientController.js
index d306dda67..f980f4450 100644
--- a/packages/react-server/core/ClientController.js
+++ b/packages/react-server/core/ClientController.js
@@ -609,9 +609,11 @@ class ClientController extends EventEmitter {
 			if (element.containerOpen || element.containerClose){
 				return; // Nothing left to do.
 			} else if (element.isTheFold) {
-				logger.time(`renderAboveTheFold.fromStart`, new Date - tStart);
-				logger.time(`renderAboveTheFold.individual`, totalRenderTime);
-				logger.time(`renderAboveTheFold.elementCount`, index + 1);
+				if (!this._previouslyRendered){
+					logger.time(`renderAboveTheFold.fromStart`, new Date - tStart);
+					logger.time(`renderAboveTheFold.individual`, totalRenderTime);
+					logger.time(`renderAboveTheFold.elementCount`, index + 1);
+				}
 				return; // Again, this isn't a real root element.
 			}