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.

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 new component: <TheFold /> #221

Merged
merged 2 commits into from
May 26, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 6 additions & 1 deletion docs/understanding-rendering.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ getElements() {
<Sidebar />
</RootElement>
</RootContainer>
<TheFold />
<Footer />
</RootContainer>
}
Expand All @@ -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.
Expand All @@ -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
Expand Down
1 change: 0 additions & 1 deletion docs/writing-pages.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
7 changes: 5 additions & 2 deletions packages/react-server-test-pages/pages/data/delay.js
Original file line number Diff line number Diff line change
@@ -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() {
Expand All @@ -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();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a big=<size> param to the test data endpoint that just tells it to return a JSON object with size keys. It's easiest to see the fold when there's a bunch of JS to for the browser to slurp up right after.

return Q.delay(ms||0)
.then(() => val||JSON.stringify({'ok':true}));
.then(() => val||JSON.stringify(big?BIG(big):{'ok':true}));
}
}
22 changes: 11 additions & 11 deletions packages/react-server-test-pages/pages/root/aboveTheFold.js
Original file line number Diff line number Diff line change
@@ -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;
}
}
20 changes: 8 additions & 12 deletions packages/react-server/core/ClientController.js
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -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.
Expand All @@ -613,6 +608,13 @@ class ClientController extends EventEmitter {

if (element.containerOpen || element.containerClose){
return; // Nothing left to do.
} else if (element.isTheFold) {
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.
}

var name = PageUtil.getElementDisplayName(element)
Expand All @@ -632,12 +634,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);
}
}
};

Expand Down
1 change: 1 addition & 0 deletions packages/react-server/core/common.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ module.exports = {
RootContainer: require("./components/RootContainer"),
RootElement: require("./components/RootElement"),
Link: require('./components/Link'),
TheFold: require('./components/TheFold').default,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Awkward to pull out .default here, but IMO worth it for the cleaner implementation in TheFold.js.

History: require('./components/History'),
ClientRequest: require('./ClientRequest'),
FramebackController: require('./FramebackController'),
Expand Down
6 changes: 4 additions & 2 deletions packages/react-server/core/components/RootElement.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
var React = require('react');
var Q = require('q');

const {isTheFold} = require('./TheFold');

const _ = {
assign: require('lodash/assign'),
};
Expand Down Expand Up @@ -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.
//
Expand Down
19 changes: 19 additions & 0 deletions packages/react-server/core/components/TheFold.js
Original file line number Diff line number Diff line change
@@ -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 = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

const isTheFold = Symbol();
TheFold.defaultProps = { [isTheFold]: true };
export function isTheFold(e) => e && e.props && e[isTheFold];

is, I think, privater

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I actually thought about that. AFAIK the reason React does that is to avoid injection attacks via JSON. I don't see that as a concern here, so I opted to just keep it simple.

_isTheFold: true,
}

export function isTheFold(element) {
return element && element.props && element.props._isTheFold;
}

export function markTheFold() {
return {isTheFold:true};
}
47 changes: 29 additions & 18 deletions packages/react-server/core/renderMiddleware.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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);
}
}

Expand All @@ -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
Expand All @@ -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(),
};
Expand All @@ -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) {
Expand Down
3 changes: 2 additions & 1 deletion packages/react-server/core/util/PageUtil.js
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -61,7 +62,6 @@ var PAGE_METHODS = {
getBase : [() => null, Q],
getBodyClasses : [() => [], Q],
getElements : [() => [], standardizeElements],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Glad I never updated the tabling around this. It's almost like I knew it was an ugly hack that should be replaced... 👹

getAboveTheFoldCount : [() => 1, _ => _],
getResponseData : [() => "", Q],
};

Expand Down Expand Up @@ -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)
Copy link
Member

Choose a reason for hiding this comment

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

If I understand correctly, this is just converting a React element to an object that looks like {isTheFold: true} for the purposes of switching on it during render, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, exactly. The normalized elements array contains only root elements and control objects.

This introduces a new control object: {isTheFold: true}.

.map(ensureRootElement)
.map(scheduleRender)
}
Expand Down