Proposal: ReactDOMServer render to Stream

[aickin]

tl;dr: I'd like to know how much enthusiasm there is on the core React team for accepting a PR with the ability to render markup to a stream. I don't need a guarantee, of course, but I'd like to get an sense if it's something the React team might do before spending a ton of time working on it.

Background

Currently, server rendering is accomplished by ReactDOMServer.renderToString or ReactDOMServer.renderToStaticMarkup, which are synchronous functions that return a string.

The server render methods can be somewhat slow. Large (~500K) web pages can easily take over 100 or 200ms to render, depending on the server. Also, since node is single threaded and single-core performance has not been improving rapidly, this is not just a problem that Moore's Law will eventually solve. The slowness in turn causes two problems in production websites with large (or somewhat large) HTML pages:

1. First, because the methods return a string, they completely render the entire component before you can return even the first byte to the browser, meaning that the browser is waiting idle while the render method is called.
2. Second, because the methods are synchronous and node is single threaded, the server is unresponsive during rendering, which means that a large page render can easily starve out renders of smaller pages (which may only take a few ms). An asynchronous render method would let large page renders yield CPU to small pages, improving average response time.

Proposed Solution

The proposal is to add two methods to ReactDOMServer, both of which return a node Readable Stream.

Stream renderToStream(ReactElement element)
Stream renderToStaticMarkupStream(ReactElement element)

In terms of behavior, these methods would return a Readable Stream that outputs the same markup as renderToString and renderToStaticMarkup, respectively.

These methods could solve both problems above. A stream can begin emitting content immediately to send to the browser, keeping TTFB constant as components get larger. Streams are also inherently asynchronous, which allows large pages to not starve out smaller ones.

A few months back I forked react into a library called react-dom-stream to do this exact thing, and it's been fairly well received (1,100+ stars on GitHub, though modest actual downloads on npm). There's also a wishlist bug thread on this repo about it. However, maintaining a fork is a less than desirable position, and I wonder if I can get it integrated in to the core code base.

My implementation in react-dom-stream is not ideal; I ended up copying basically every implementation of mountComponent and making the copied versions asynchronous. The benefit of this approach was that it didn't affect renderToString performance, but the obvious drawback was that any code changes or bug fixes would have to be performed in both forks of the code, which is not ideal.

Recently, I figured out a way to share code in a better way between renderToString and renderToStream, but the result was that renderToString slowed down relatively significantly (+30%, plus or minus). I'm trying to figure out how if I can tune it more carefully, but it's not looking great yet.

Questions

So I think the questions I'd like to ask to the team are:

1. Assuming it were perfectly implemented with no performance degradation of the current code, is this even something you are willing to accept a PR for this feature?
2. If you were to accept this, how important to you is it that the string and stream versions of the code follow the same code path?
3. Are you willing to accept any degradation in performance of the string version of server rendering in order to get stream rendering that has constant TTFB and keep a single code path?

Thanks for your time, and thanks for all the great work you do on react!

(Tagging @spicyj, @gaearon, and @sebmarkbage, as I was instructed to do on the react IRC channel.)

[sophiebits]

Thanks for following up and filing this.

I haven't looked at your code in detail, but I am generally positive on this. It sounds like serious users of server rendering (which include none of us on the core team) would all appreciate this feature. We should get feedback from some of those people before settling on an API.

I would prefer to have parallel codepaths rather than degrade the performance of renderToString. It shouldn't require serious changes: I believe that React[Server]ReconcileTransaction and ReactDOMComponent are really the only modules that have knowledge of the server-rendering difference, so there shouldn't be much to split. In particular, none of our "isomorphic" modules know about server rendering because it is a DOM-specific concept (ex: React Native does not support server rendering).

[mridgway]

At Yahoo, we do a significant amount of server rendering with React and are extremely interested in getting streaming support in the core. As such, I am more than willing to help out in any way possible.

I have to done some investigation into render streaming and also looked at your implementation of react-dom-stream. I agree with your assessment that as it stands it is difficult to support streaming without modifying a lot of the code in React*Component's mountComponent method. This is because the recursion of children is implemented within this method and you would have to have checks for the current transaction in each of the mountComponent methods for returning a string vs. writing to a stream.

As such, I think the best course of action (and recommendation by @sebmarkbage) was to move the recursion out of the individual React*Component.mountComponent methods and into the ReactReconciler.mountComponent method instead. This method would then receive arrays of metadata: strings and child components that could be written to the transaction or recursed. The transaction would then take care of concatenating strings or writing to a stream via a common interface.

I did some initial work on hoisting the recursion in #6011. Centrallizing the recursion as in this PR would make it much easier to support streaming and asynchrony in the future.

It is difficult to commit to working on this further without buy in from the core team and more information on what is involved in #6170.

[aickin]

@spicyj:

I am generally positive on this. It sounds like serious users of server rendering (which include none of us on the core team) would all appreciate this feature.

So great to hear, thanks!

We should get feedback from some of those people before settling on an API.

Agreed. Other than this issue, and outreach I will do to react-dom-stream users, are there any other methods to get feedback you would suggest?

I would prefer to have parallel codepaths rather than degrade the performance of renderToString.

Good feedback, thanks.

I believe that React[Server]ReconcileTransaction and ReactDOMComponent are really the only modules that have knowledge of the server-rendering difference, so there shouldn't be much to split.

I think it's a bit more widespread than the code that has knowledge of server-rendering; the issue is that the mountComponent and mountChildren signature (everywhere it shows up, both in DOM-specific and non-DOM-specific code) is synchronous, which won't work with streams. In my current implementation, I wrote a parallel set of mountComponentAsync/mountChildrenAsync methods which have an async function signature.

I'm currently working on a newer version where mountComponent instead returns a data structure that can be processed either synchronously or asynchronously, depending on the caller. The preliminary (pretty messy) code for it is here if you are interested (ignore the test code; most of the interesting stuff is in React***Component and StringLazyTree files).

@mridgway:

also looked at your implementation of react-dom-stream

Eek. The implementation is... not my proudest achievement; I had to rewrite it from scratch about 4 or 5 times (callbacks, generators, async/await) because perf was often abysmal once I got it working.

I think the best course of action (and recommendation by @sebmarkbage) was to move the recursion out of the individual React*Component.mountComponent methods and into the ReactReconciler.mountComponent method instead.

I think I did something like this in my most recent experimental WIP branch (as I said above, most of the interesting stuff is in React***Component files and StringLazyTree). Rather than hoisting the recursion into ReactReconciler, though, I pulled it out into a structure I called StringLazyTree (after DomLazyTree), but the rest of the idea is basically the same. The mountComponent/mountChildren methods now return a StringLazyTree, which is basically an array of raw strings and closures to call as you recurse down the tree. That can be done synchronously or asynchronously, which preserves the same code path for renderToString and renderToStream.

This experimental branch passes nearly all the tests (it doesn't do checksums yet, but I know how to do that from previous work), but it's about 50-100% slower for last byte than renderToString in my tests. Working on that now, although I'm not making much progress.

It is difficult to commit to working on this further without buy in from the core team and more information on what is involved in #6170.

Thanks for pointing that out; I didn't know about it at all. Is there any info on what the algorithm rework looks like?

[goatslacker]

Tentative plan:

1. Get rid of checksum checking on the client and checksum generation on the server. This has the benefit of loosening up the guidelines for how markup is decided to be valid or not on the client when re-rendering. The new approach would walk the DOM tree and verify that the tree is the same, attributes and their values are the same, and content is the same.
2. Get rid of data-reactid. We can keep track of DOM -> internal tree while we walk the DOM on client render.
3. Split out client rendering and server rendering. Now we can have server renderers that exist outside of React and can be independent.

Does this sound right? Did I miss something?

Other notes:

• We need to ensure that if we do have multiple server renderers that live outside of React that React is aware of their existence and runs their tests. I'd like to know about breaking changes ahead of time.
• Perhaps we should provide a documented spec of what is expected from a server renderer. What lifecycles should we call? What the DOM markup should be like? etc...

@jimfb @aickin and others who'd like to comment.

[sophiebits]

The new approach would walk the DOM tree and verify that the tree is the same, attributes and their values are the same, and content is the same.

This would probably be very slow and would likely require a bunch of internal refactors to even make possible.

Get rid of data-reactid.

We barely need this at all now but I kept it intentionally so we can distinguish React-rendered nodes from nodes from elsewhere (from browser extensions, for instance).

[jimfb]

We barely need this at all now but I kept it intentionally so we can distinguish React-rendered nodes from nodes from elsewhere (from browser extensions, for instance).

Nodes from elsewhere are going to disturb a react render anyway, and thus get blown away when React mounted into the dom. I don't think there is much value in knowing where the nodes came from (knowing that the nodes differ is sufficient), but there is a disadvantage to being forced to maintain the data-reactid because it mandates a specific render order.

[aickin]

Does this sound right? Did I miss something?

That's what I remember, yes.

This would probably be very slow and would likely require a bunch of internal refactors to even make possible.

We talked about this a while, and we weren't entirely sure how slow it would be, but I agree it's the biggest risk in the idea. @jimfb argued that it would be at least same big(O) of what is done currently when rendering on top of server markup, but the question is how much worse reading dom tag names and attributes is than constructing a giant string. I'm worried that it might be MUCH worse, but we will see.

[goatslacker]

likely require a bunch of internal refactors

This, and slow, is what worries me mostly. Any pointers on where to start looking?

Also helpful would be knowing where the logic that checks if it's React.createClass, React.Component, or SFC is at.

[aickin]

@goatslacker also, thanks for writing this up! 👍

FYI, I'm trying to hack my way through a rough cut of step 1 today. I'm in the middle of it right now, and it's definitely a beast. I've got a few dozen new unit tests passing but a bunch not.

[jimfb]

@goatslacker

Also helpful would be knowing where the logic that checks if it's React.createClass, React.Component, or SFC is at.

src/renderers/shared/reconciler/ReactCompositeComponent.js (eg. #5884)

[lelandrichardson]

I think an important piece of this is building a rough spec + test suite that can be run against external renderers that is also run against react core to ensure that breakages are properly communicated before being published. This should cover:

1. lifecycles that are expected to get called during render
2. that react on the client can properly bootstrap itself on top of nodes with the proper structure (accentuating the differences in character-by-character html output that are possible)
3. obviously covering a lot of the corner-case HTML oddities
4. (am i missing anything???)

This could be done similarly to ECMA's test262 repo, and breakages + additions could be communicated inside of the github repo that holds these tests.

This is really exciting!

[aickin]

Quick update on potential speed of reconnecting to DOM by walking the dom rather than checksum (step 1).

I have a branch that does this now, although it's by no means complete (needs work on interactivity, a bunch more unit tests, 3 existing unit tests fail, and the code is not as readable as I'd like). It is complete enough to do a really rough comparison of performance between the two strategies, though, which I started this morning.

My very preliminary and very rough observation is that DOM walking is about 20% slower than checksum in Safari/OSX and about 40% slower in Chrome/OSX.

This is obviously not ideal (faster is better!), but it's also not an order of magnitude increase. It's also not a giant amount of absolute time: in my (once again, rough) test on a 635KB render, the difference amounted to 20-40ms. When I tested a 107KB render, I couldn't detect a difference between the two result with ad-hoc testing. It's also worth noting that I haven't done any perf optimization on the branch.

I'm still concerned about the potential for bad perf regression, particularly given that different browsers can have wildly different perf characteristics. I think the best path forward is to polish up my test, make it automatable via Selenium, and run it enough times to get statistical significance on a variety of platforms. I think good coverage would look something like:

• Multiple document sizes
  • ~25K
  • ~100K
  • ~500K
• Platforms
  • Chrome/Windows 10
  • Firefox/Windows 10
  • OS X latest/Safari
  • iOS 9.2/Safari
  • Android 4.4/Default Browser
  • IE11/Windows 10
  • IE10/Windows 8
  • IE9/Windows 7

Does that make sense? Do folks agree?

Also: How much perf regression on client reconnect to server markup would you accept, if any?

[lencioni]

Great work! Getting better numbers makes a lot of sense to me.

How much perf regression on client reconnect to server markup would you accept, if any?

I think the answer might depend, at least partly, on how much faster we think we can make server rendering with the optimizations we discussed.