Skip to content
This repository has been archived by the owner on Jun 3, 2019. It is now read-only.

Upgrade to React@16 (next) #479

Closed
ctrlplusb opened this issue Jul 26, 2017 · 32 comments
Closed

Upgrade to React@16 (next) #479

ctrlplusb opened this issue Jul 26, 2017 · 32 comments

Comments

@ctrlplusb
Copy link
Owner

And eventually replace react-async-component with native async behaviour (hopefully!)

facebook/react#10294

@ctrlplusb
Copy link
Owner Author

Gonna. Change. Everything.

@birkir
Copy link
Collaborator

birkir commented Jul 26, 2017

I can ... not ... wait! 😸

@oyeanuj
Copy link
Contributor

oyeanuj commented Jul 26, 2017

I'm excited about the SSR with streaming!

@rajsek
Copy link

rajsek commented Jul 27, 2017

Going to try and blog my experience

@strues
Copy link
Collaborator

strues commented Jul 27, 2017

Looks like async server stuff isn't coming until later. Either way, cant wait.

@diondirza
Copy link
Contributor

Awesome, this gonna be a major improvement.

@ctrlplusb
Copy link
Owner Author

Yeah, @strues is right, looks like async work didn't land into the new server renderer.

Also the server renderer has a few issues and sounds quite experimental at this stage so it may be worth sitting this out a bit in a branch.

The server renderer doesn't yet support returning arrays and strings from components.

The server renderer has been completely rewritten, and now offers a streaming mode (it's currently exposed via react-dom/node-stream). Server rendering does not use markup validation anymore, and instead tries its best to attach to existing DOM, warning about inconsistencies. This server renderer code is still very new, so it is likely to have issues. Please report them.

@ctrlplusb
Copy link
Owner Author

One cool new feature of fiber is that it is gonna be less heavy handed on the checksum validation! Allowing partial mismatches which it would try to resolve in place rather than doing a full re-render. Sounds like an awesome win for SSR as sometimes it can be tediously difficult to keep the checksum matching in place.

@oyeanuj
Copy link
Contributor

oyeanuj commented Aug 5, 2017

With regards to next, curious what you folks think about https://github.com/faceyspacey/react-universal-component or other such newer components coming up for universal rendering? Or is react-async-component/react-loadable working smoothly for you folks?

@ctrlplusb
Copy link
Owner Author

ctrlplusb commented Aug 5, 2017

Hey @oyeanuj - I haven't looked at it in detail yet, but it sounds very promising.

Here is my brief overview of the 3:

react-loadable is the library of choice if you are doing client side only. Community behind it and James is a very pragmatic and respected developer.

react-async-component was my temporary solution to being able to do SSR with RR4. It gets you most of the way whilst avoiding some of the crazy webpack/babel configs that I previously had to do.

react-universal-component looks like @faceyspacey has nailed out most of the issues and complexity you have around webpack config in terms of SSR, and abstracts it all away. If it gives us sync rendering on the server with a fluid dev experience thats a win in my opinion and we should consider switching.

And then a final note/feeling on all of it:

Ideally I would like a more native implementation, i.e. async rendering in the server render of react, however if react-universal-component may be another respectable stepping stone whilst we wait for that to happen.

@faceyspacey
Copy link

faceyspacey commented Aug 6, 2017

When Fiber async comes out, apparently there still won't be support for it:

https://twitter.com/dan_abramov/status/893267299909554176

However, even if there were, that doesn't solve the primary problem: discovering what chunks that were rendered. Why? Because on the client, the initial main chunk etc still has to load, be parsed, evaluated, and then executed/rendered before additional chunks were discovered and fetched from the server.

So you just lost somewhere between, i dunno, 500ms and 5 seconds, via a completely async strategy. That said, if the chunks were discovered in the async rendering on the server, and available for the async rendering on the client, they would already be loaded, parsed and evaluated at the same time as main before the first rendering, and available in one tick after the async import().

So in essence even if Fiber async massaged the process here, it only removes one (now irrelevant) challenge: "weakly" requiring dynamically imported modules so they don't end up in the parent bundle. It's irrelevant now because of babel-plugin-universal-import, but would certainly be welcome if that wasn't needed. It's just not the primary challenge, though an important one to making this all frictionless and therefore accessible for wide-spread use.

Either way, it doesn't sound like 2017 will see React resolving promises in render for you. Also I am hesitant about the performance implications of waiting during render. Maybe it's a good thing to unblock the thread for other requests to come through, and maybe it's just best to complete that render and get the result to the client. I don't think we'd be filling our render tree up with promises unless we absolutely had to, which we now don't. But honestly I could be wrong on that one, as after all the non-blocking thing is what Node perf is all about.


I'm very excited about the new streaming aspect. I wonder if async rendering on the server jives with the streaming aspect.

On a side note, the streaming strategy has its own core challenge to code-splitting: since stylesheets come at the top of the page, you have to know what stylesheet chunks to render at the start of the stream, not as you discover them midway through the rendering. By that time it will be too late.

The solution I and @swernerx have come up with is to move the discovery of what chunks will be rendered to the "route level." For example, if we know that route/path /foo/:bar always requires the same chunks, we can know the stylesheet chunks to append to the top of the page at the beginning of the stream.

There's 2 ways we've thought of thus far to address that:

  • when you define your UniversalComponent in the top level closure evaluated before render (what some people refer to as "statically"), also define the routes they apply to, eg: universal(import('./Foo'), { route: '/foo/:bar' }). That way before renderToStream is called or at the beginning of its execution, those stylesheets can be known.
  • the second is to wrap renderToString and renderToStream in a thin wrapper function that calls renderToString the first time a new route is requested, and based on the chunks discovered, automatically embeds them in future requests when renderToStream is used. It would cache a hash of routes to arrays of chunk IDs.

At the end of the day, there's a lot more that needs to be done in this space besides just serving chunks. Serving data fetched at the component-level and then rehydrating on the server is one. The next is an interface for the client to prefetch those component-level data dependencies. That's not something I'd expect to see React helping us with any time soon. And they don't need to--it can all be done in package-land. @ctrlplusb 's react-async-bootstrapper which does most of the work has helped me build such a prototype already. It was built on the 1.0 version of react-universal-component, so I primarily need to make it work for the 2.0, but also address future considerations such as prefetching before releasing the API.


One final thing: @ctrlplusb what do you think about this: if during the initial/additional react-async-bootstrapper render that only occurs within the virtual dom, we actually do some of the work of renderToString, caching the result. That way when renderToString is called, it basically just concats strings or perhaps renders one big object that was already created, i.e. renders to string the result of one single very big render function, with much of the work already done. That would basically be renderToStringAsync. Thoughts??

@oyeanuj
Copy link
Contributor

oyeanuj commented Aug 29, 2017

@faceyspacey I'd love to be able to try the prototype of this data-fetching library, if available!

Serving data fetched at the component-level and then rehydrating on the server is one. The next is an interface for the client to prefetch those component-level data dependencies. That's not something I'd expect to see React helping us with any time soon. And they don't need to--it can all be done in package-land. @ctrlplusb 's react-async-bootstrapper which does most of the work has helped me build such a prototype already. It was built on the 1.0 version of react-universal-component, so I primarily need to make it work for the 2.0, but also address future considerations such as prefetching before releasing the API.

@ctrlplusb Have you or anyone already integrated react-universal-component with this library? Has it been easy/hard?

@leidegre
Copy link

leidegre commented Aug 30, 2017

...Fiber aka asynchronous rendering in React, is just a scheduling algorithm. It just allows React to interrupt itself and do something else if it recognizes that a newer, more prioritized component update will replace part of the DOM tree in the future, then it can abort, not wasting any more resources there and start working on the newest change. It ought to give better-perceived perf when many things change over a period of time.

So, I'm kinda lost about how async-component which is a code splitting strategy is related to that? 😃

Also, I've found that the simplest way to actually get this to work, is just to be honest about the fact that you need to know your routes up front (not necessarily statically, just up front). The way we have it set up is that we just walk our routes config.

We then use matchRoutes and renderRoutes from react-router-config to discover what to draw. At this point we know what components we need to prefetch so there's no need for multiple render passes or component life-cycle hooks.

I thought the idea of having an async-component as a higher order function was swell but I changed it a bit. As part of our route discovery process we unwrap all the async components in the matched routes. It's very simple really.

const routes = [
  {
    path: "/",
    component: asyncComponent()(() => import("./Actual"))
  }
];

Promise.all(
  matchRoutes(routes, "/").map(route =>
    route.component
      .resolve()
      .then(component => (route.component = component) /* <- unwrap */)
  )
).then(() => render(renderRoutes(routes)), document.getElementById("..."));

resolve is just a static function that does the dynamic/async import and returns a promise with the component. Internally it avoids calling the import directive more than once and it does have a fallback for client side rendering.

I think that's right, hope it conveys the general idea. By the time we actually do render, client and server, the components will have been resolved from import directives to just components. And it works really well.

@faceyspacey
Copy link

faceyspacey commented Aug 30, 2017

I'm a big fan for doing things "route level" instead of "component level" as that solves a lot of problems with SSR. Basically if you have nested components doing additional async fetches, you are greatly increasing load time on the server. And if you only have one such component that you discover per route, you're better off formalizing the contract with Routes as I promote with redux-first-router.

However, the challenge in doing what you're doing here without a synchronous fallback when available is that the time till interactive (TTI) is way longer on the client. You have to wait for all the following to sequentially happen:

  • download the main chunk
  • parse it
  • evaluate it
  • render it
  • discover + load imports
  • re-render!

If your components, or in your case routes, also had webpack's require.resolveWeak functionality as is built in to react-universal-component you could discover the chunks needed and transport them to the client by embedding them in the initial HTML. The result is you don't have to wait all the way to the render to get your remaining chunks. Here's what the above sequence now looks like:

  • download the main chunk + all rendered/discovered chunks server-side
  • parse everything
  • evaluate everything
  • render once

So again, I love your routes setup. I really recommend that way to go when it comes to SSR. But in terms of code-splitting, you're losing precious time (basically seconds) before the app can be fully used. Here's the beginning of how I would do your approach (it would need the infrastructure from react-universal-component and other packages from the Universal suite such as webpack-flush-chunks re-written for a route-level approach):

const routes = [
 {
   path: "/",
   componentAsync: asyncComponent()(() => import("./Actual")) 
   componentSyncId: require.resolveWeak("./Actual")
 }
];

Promise.all(
 matchRoutes(routes, "/").map(
   route => (route.component = route.component.resolve())
 )
).then(() => render(renderRoutes(routes)));

elsewhere:

const isServer = typeof window === 'undefined'
const render = ({ app, chunks } ) =>
  isServer
     ? res.send(html(app, chunks)) // injects discovered chunk <scripts> into served HTML
     : ReactDOM.render(app, document.getElementById('root'))

// pseudo-ish code:
componentRoute.prototype.resolve = function() {
   if (this.hasChunk()) return Promise.resolve(this.componentSync())
   else return this.componentAsync() // already a promise
}

componentRoute.prototype.hasChunk = function() {
    return !!__webpack_modules__[this.componentSyncId]
} 

componentRoute.prototype.componentSync = function() {
    // very fast (1 tick), 
    // but still a promise (it could be written to be completely sync though)
    return asyncComponent()(() => Promise.resolve(__webpack_require__(this.componentSyncId))
} 

In this case, resolve will somehow dynamically toggle between the two based on detecting if hasChunk has the chunk.

And again, ideally the sync version doesn't have to be wrapped in Promise.resolve. React-Universal-Component does not do that. When possible, it renders synchronously. I only
made it consistently promise-based here for the purpose of a quick example.

What's missing is that renderRoutes() needs a way to discover the names of the chunks that correspond to componentSyncId so it can embed the additional <script> tags. The ID is just the webpack module ID of a given a module. The chunk name is different. You can look into the webpack-flush-chunks code for inspiration. But ultimately there can be a lot to it. My babel-plugin-universal-import package actually generates the chunk names in an easy to discover way where react-universal-component is able to record it and pass it along to webpack-flush-chunks, etc etc. It's very annoying stuff.

..Or instead of making my system work with Routes, just rely on react-universal-component. See, the thing is this: since RUC is able to synchronously require on the server (and more importantly again on the client), it doesn't matter if you have nested imports--they will all resolve synchronously (and therefore quickly/instantly) in these 2 key places. You don't save any time [till interactive] by doing it at the route level. Essentially you get nothing for your money by moving universal/dynamic imports to the route level like you would with async data fetches, since the components render synchronously like any other component on initial load :)

One final note: overall the idea is to go back to the way traditional web apps render. But with code-splitting now on the table. So traditional web apps have SSR inherently, and on the client, the scripts just expect to render synchronously one after the other. Things are synchronous, and faster as a result, everywhere essentially. When you introduce code-splitting, everything goes haywire. Via the proposed approach, the universe is back into alignment, just with code-splitting now integrated. No, additional potentially recursive promise resolution on the client. And most importantly: no multiple stages of scripts parsing/evaluating/rendering.

@faceyspacey
Copy link

@oyeanuj we'll chat on Reactlandia Chat about it. Ultimately the amount of time I have has gone to near zero since the release of these packages. It may take significant time to bring the old prototype back to life. I rather just get the new one out as soon as possible. Ultimately, I really need help with all this stuff to take it to the obvious next levels. There's a clear vision here. One that has been needed for a very long time in Reactlandia.

@leidegre
Copy link

leidegre commented Aug 31, 2017

@faceyspacey

if you have nested components doing additional async fetches, you are greatly increasing load time on the server

Not necessarily. The route matching algorithm is a discovery algorithm and it can run before any of the asynchronous tasks finish, so it's should only matter what the latency is for the longest running async task. We've tried to be vigilant about making sure everything which adds latency is done in true parallel fashion.

I'm still assuming that if you do choose to nest routes and you're still up front about it.

const routes = [
  {
    path: "/",
    component: asyncComponent()(() => import("./Actual")),
    routes: [...] // <- nested routes go here
  }
];

the challenge in doing what you're doing here without a synchronous fallback when available is that the time till interactive (TTI) is way longer on the client

What's a good metric? I think we do pretty good, 50-500 ms depending on how we measure and what extensions run in the background. Lots of noise.

Again, a bit of bookkeeping is needed for this but we actually name all our chunks explicitly. I did try to figure out a more magic way to make this work but it's just too much pain to go thourgh, it's brittle and difficult to understand.

const routes = [
  {
    path: "/",
    webpackChunkName: 'chunk1',
    component: asyncComponent()(() => import("./Actual")),
    routes: [...] // <- nested routes go here
  }
];

So if you hit this route, we know that we should prefetch chunk1 which we do. The only thing the client has to do is to resolve the promises there is now latency there other than the implied setImmediate, it should be less than 1 ms, for sure.

We don't embed the javascript together with the actual first request but that could be even faster, will have to test that out later.

Anyway, my journey through this has led me to conclude that it's just easier to keep all this in a configuration up front. And rely on a recursive renderRoutes function to render what's needed and where needed.

My biggest pain right now is the CSS file, it's just a huge chunk, haven't been able to do anything about that.

@faceyspacey
Copy link

faceyspacey commented Aug 31, 2017

my extract-css-chunks-webpack-plugin handles that.

I highly suggest you read my articles. I've solved all these precise pain points in essentially the best possible way.

https://www.medium.com/faceyspacey

Are using Redux? I have 2 "beats" if you will: Universal Rendering and my Redux-First Router product. They work together:

https://github.com/faceyspacey/redux-first-router

In short, there is a connection between routing and universal code-splitting, but that is actually only "prefetching." Prefetching potential chunks to be used like Next.js. Otherwise there isn't, and u dont need to add this complexity to your routing mechanism. However these 2 product lines work great together, while really getting routing right for Redux.

There should be no pain around this stuff if you're using these tools. If you're using React Router + Redux, don't. Redux-First Router is, as several of my users have said "the router now."

Sorry if im comin off abrupt, Im just laying it out there--cuz all this would mean u have nothing custom to maintain anymore, and ur up to speed with the latest and greatest. Read the articles--u sound extremely versed in this stuff. I think if u read the articles on my medium publication, u'll find a glimmer of happiness in this space (which has been a horrible ugly place to live for so long).

...I got it though, ur embedding the chunks in the initial request. That was the primary hurdle. Great work!

So either way, if u research what I've setup, u'll see i also made https://github.com/faceyspacey/babel-plugin-universal-import

which imports both a js chunk and stylesheet. and the extract css chunks webpack plugin insures you generate multiple stylesheets. It's all a system. It's basically dubbed as "the frameworkless approach to the best features of Next.js." Which means there is some configuration/setup, but that's the price of forgoing the limitations of a framework.

Anyway, all this makes everything work as u would expect with traditional web apps, just with code-splitting adding to the mix. I'm not saying it's the only way, but truth is there has been no general frameworkless way. I'm supporting all my users vigilantly. Feel free to come chat with me in "Reactlandia Chat" linked from the top of my repos. ..You could implement this stuff in your app and have it no longer be your responsibility as a custom ad-hoc setup. Hope you come to my camp :)

and thanks @ctrlplusb for all the great recommendations! It's truly a rare thing to see a developer having made similar tools recommend the work of another. Ur a gem!!

@leidegre
Copy link

leidegre commented Sep 2, 2017

@faceyspacey What's makes you think I haven't? ;D

But it's not that easy, we know our needs and we're adapting what's there to fit and with less abstraction. I really prefer not to hide these details or rely too much on external packages for core things. I've read through many of your articles and drawing inspiration from looking at your work. Sometimes though, it's not entirely obvious how we'd pull everything together and to be honest, we just don't have the luxury to fix everything right now. Next time around, we'll make a pass over CSS again and by that time I'm hoping the webpack support is there.

@oyeanuj
Copy link
Contributor

oyeanuj commented Sep 2, 2017

@leidegre For the code you mentioned here: I was wondering if you plan to publish it anywhere? Or is there a gist/repo with rest of the server/route logic that one could take a look at?

@faceyspacey
Copy link

@leidegre that u mentioned ur CSS is one big chunk. Yes u may be waiting a lot longer than hoped for full webpack support.

Extract CSS chunks webpack plugin and the dual-import package can work with ur route based strategy. That way u can forgo the bigger component based change. Client side it should just work. Server side it sounds like u already have under control--but you will have to modify it to send stylesheets (which probably can just share the same name as the js chunk but with a different extension).

Check out babel-plugin-dual-import. It's for use without React Universal Component.

@leidegre
Copy link

leidegre commented Sep 3, 2017

@oyeanuj oh yeah, I'm not done with this yet but everything is there you can take a look here https://github.com/tessin/tessin-react the function that does that work is called unwrapAsyncComponent. You can search for that as a reference point in the repo. Just a heads up, I'm going to reorganize the repo a bit and it's something we use as an internal tool, I don't mind sharing it but it's not something I'm planning on maintaining for people outside our organization.

@faceyspacey I would love to pick your brain about this subject at some hypothetical future point in time...

@williamoliveira
Copy link

williamoliveira commented Oct 10, 2017

So what is the status on this? anything holding 16 back? Are we gonna see that that sweet renderToNodeStream TTFB soon?

EDIT: nevermind, just saw its on next branch, just no renderToNodeStream tho 🤔

@ctrlplusb
Copy link
Owner Author

Yeah, it's in progress. 😉

@alex-shamshurin
Copy link

Any update?

@mschipperheyn
Copy link

@ctrlplusb is it work in progress: it doesn't work yet or is it work-in-progress: not everything there yet, but what is there works? Eager to move to React 16!

@vsc-github
Copy link

One of my favourite starters, has everything a real in-production react app would need. Would love to have react 16 features. Please let us know, if we can help. :)

@alex-shamshurin
Copy link

Hmm, guys, React 16 is almost the same (externally), we moved a big project to it for one hour. What is a problem to upgrade?

@birkir
Copy link
Collaborator

birkir commented Jan 19, 2018

@alex-shamshurin not a big deal, but a lot of stuff was flakey.

We have 16 working in a opinionated fork here: https://github.com/ueno-llc/starter-kit-universally

ueno-llc/starter-kit-universally@e9915fe

@oyeanuj
Copy link
Contributor

oyeanuj commented Jan 30, 2018

I have been using the next branch, and it (and React 16) seems to be working well for me. Are folks seeing particular issues?

@oyeanuj
Copy link
Contributor

oyeanuj commented Feb 2, 2018

one thing which I just realized trying to upgrade both Styled-Components and React-Helmet (rather React-Helmet-Async) to streaming is that this file will need to be reworked to enable streaming of content within head and body.

https://github.com/ctrlplusb/react-universally/blob/master/server/middleware/reactApplication/ServerHTML.js

Right now, the whole app gets streamed, after collecting head and style tags whereas both React-Helmet-Async and Styled-Components now allow for streaming their respective output.

React-Helmet-Async: https://github.com/staylor/react-helmet-async
Styled-Components: https://www.styled-components.com/docs/advanced#streaming-rendering

@mschipperheyn
Copy link

I'm noticing that the react middleware still uses renderToString for the entire React App and then renderToNodeStream for the HTML using that appString. That looks wrong. renderToString will create a performance bottleneck as far as I understand it..

@diondirza
Copy link
Contributor

Correct, and we already have PR #564 to solve that issue.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests