Export render function

Created by: pugnascotia

In order to facilitate server-side rendering (#1100 (closed)), change the production webpack config to package the bundle as a UMD module, making it usable in a number of environments. The module's name is derived from the application's name in package.json.

For this to be meaningful, the bundle's entrypoint must export a function, so I've added a basic render function to index.js. The existing browser mount must be wrapped in a guard condition to stop it failing in a server environment.

Created by: kpuputti

I tested this approach and it works nicely!

However, a server rendering setup will most likely not render the whole application with no props, but integrate with the application router with the URL given to the server. Would this require CRA to bundle e.g. React Router and this render function somehow integrate with it, or how could this problem be solved?

Created by: kpuputti

Sorry, after reading the linked issue, I realised that the whole setup can be done in the application! So this approach would work nicely with any routing setup.

Created by: kpuputti

Would this break chunk loading since the default Webpack target is "web"?

Created by: pugnascotia

Chunk loading behaviour would remain unchanged, though you'd still need a strategy to cope with instances of e.g. require.ensure on the server. For example:

https://github.com/ryanflorence/example-react-router-server-rendering-lazy-routes

Created by: pugnascotia

Any feedback on this?

Created by: pugnascotia

@gaearon Any chance of this getting merged?

Created by: alex-at-oclc

I like the approach of supporting both client and server from the same build. By sharing the Webpack config it introduces a few compromises - for example, you can't access the runtime process.env on the server. Nothing that can't be worked-around though.

I'd prefer to add src/server.js as a second (optional) entry point in the Webpack config, leaving the existing src/index.js unaltered and clean for those uninterested in SSR. For most SSR apps, the exported render function is going to need the request and to send the complete response. For example, react-router needs the path from the request, and may respond with non-200 status codes. Helmet needs to write to the document head. Redux needs to write the initial state somewhere.

I have a similar setup working, also with SSR and hot-module-reloading in the dev server - albeit currently with a separate server compile. I got some ideas from https://github.com/facebookincubator/create-react-app/pull/172, but managed to get it working all in a single WebpackDevServer in the same process. I added a serverSideRender flag to package.json - without this set, the CRA behaviour is unaltered and still beginner-friendly. Setting it to true enables SSR in the dev server and the build. One other thing I do is to pickup the asset manifest and pass that into the render function, so that it knows where to look for the bundle files. There's no coupling to anything other than node-fetch on the server. Most of the changes are to scripts/start.js.

Is it worth me cleaning that up and submitting a PR? I'm unclear whether there is a fighting chance of getting something like this accepted.

Created by: pugnascotia

My impression (and I hope @gaearon will correct me) is that CRA is not intended to be an all singing, all dancing boilerplate - there are others more aligned with that goal. The reason that the UMD bundle change is more reasonable is because it's relatively non-instrusive and leaves the heavy lifting of SSR to the developer. All the coding around routing and redux state is do-able with the UMD bundle as proposed - I know because I have a branch of my own boilerplate that uses it, and does both routing and state. But hey, that's just my opinion.

Created by: gaearon

Hey, sorry for no reply. I intend to review this, but as you can see by other PRs, there’s quite a queue to review so it will take a while.

I’m very busy with work on React itself right now so I will ask collaborators to look at PRs. New features are likely to be reviewed later than bugfixes.

We’ll come back to this, but not right now. Thanks!

Created by: pugnascotia

OK, thanks for the update.

Created by: pugnascotia

I've rebased off master and tidied up a couple things, no biggies.

Created by: alex-at-oclc

@pugnascotia: I agree with your comments. I've not gone that much further than you - all of the routing, state management etc. is still left to the application code. The key difference is that I have the dev server automatically calling the render function (if serverSideRender is true) - which requires a standard function signature. I think it's valuable to run SSR in dev mode, as it helps early identification of broken SSR or when SSR renders different content to the browser.

That aside, your PR delivers a lot of value with very little impact.

Created by: pugnascotia

Hello maintainers,

I've rebased the branch on master again, and written a basic end-to-end test script, mostly by copying the existing 'simple' script.

I've also updated the documentation to cover the changes. Feedback welcome!

Created by: Timer

I believe not having to eject is crucial here when using code splitting because we're going to be pushing it hard in 0.10 via webpack performance hints. Can we exclude export default function render() { return renderToString(<App />); } by default and detect when it's added, which auto generates the related files for server side rendering (including the second webpack bundle)?

Created by: pugnascotia

I'm up for changing that, can we thrash what we need? How about:

1. Detect either an ES6 default export or a named export of render from the bundle.
2. If present, make the bundle a UMD module (to allow non-Node, non-code-splitting users to still do SSR)
3. Generate a second webpack config, which must: 3.1 Set the target to node 3.2 Use a different output folder - not sure what to call this, how about server-build? Or the current contents of build could be moved to build/client and the server bundle could go to build/server. Looking for input here. 3.3 Other changes will likely be required. For example, the server build should probably consider only JavaScript resources - everything else should handled exactly as-is by the current production build.

Exclusions:

1. CommonsJS exports are ignored
2. We won't provide anything to actually execute the server render.

Created by: gaearon

I’m growing hesitant about this, though I really appreciate the effort that went into it. Mostly because the landscape has changed since our original discussion.

There are a few things that concern me:

• Reliance on parsing as a hint (IMO it’s a bit too implicit even by our standards).
• Dynamic import() in Webpack 2 doesn't have a very good story for server side rendering (AFAIK).
• Mostly, I’m concerned that we’ll ship a simple-but-not-good integration, and our users will miss out on all the great things that Next.js does.

In the meantime, Next.js has grown more popular, keeps getting very non-trivial optimizations, and has integrations that are familiar to our users.

I think that, at this point, it might be better to promote Next.js more heavily, and just embrace that this is the problem space they understand better.

What do you think?

Created by: Timer

I'm a little conflicted. I believe we can do some great things in this space, and I don't think parsing out an exported render is too implicit.

IMO Next.js is very opinionated (for good reasons), but I think generic support in CRA would be appreciated by many. Dynamic import() works afaik, but simply doesn't resolve synchronously which means SSR will show "loading" components. I believe this is a fair trade off when people can use react-async-component or the newer react-loadable. We could even hint this when using SSR.

I think it'd be fair to at least hold off until 0.11 instead of closing it so we have longer to evaluate the pros & cons.

As for the choice of espree, would it not make more sense to use babylon since we already depend on babel? This would make sure our modules don't grow and we don't have divergence of language features in the future.

Created by: gaearon

I think it'd be fair to at least hold off until 0.11 instead of closing it so we have longer to evaluate the pros & cons.

Sounds good.

As for the choice of espree, would it not make more sense to use babylon since we already depend on babel?

Is there a downside to a separate file like src/server.js?

Created by: Timer

Is there a downside to a separate file like src/server.js?

There's nothing wrong with this other than requiring some (possibly) duplicated code; it's probably a bit more explicit (which is better).

Created by: pugnascotia

So would the idea with src/server.js be to use it as a separate Webpack entry point, and generate the server bundle that way?

For dynamic imports, I've been experimenting with defining an extra CRA Babel package that pulls in babel-plugin-dynamic-import-node. This just turns dynamic imports into a regular requires that satisfies a promise. That should make it possible to write server-side code that can render a complete page. Thoughts?