Uh oh!
There was an error while loading. Please reload this page.
Export render function - #1292
Conversation
kpuputti
commented
Jan 4, 2017
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? |
kpuputti
commented
Jan 4, 2017
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. |
kpuputti
commented
Jan 4, 2017
Would this break chunk loading since the default Webpack target is "web"? |
pugnascotia
commented
Jan 5, 2017
Chunk loading behaviour would remain unchanged, though you'd still need a strategy to cope with instances of e.g. https://github.com/ryanflorence/example-react-router-server-rendering-lazy-routes |
pugnascotia
commented
Jan 15, 2017
Any feedback on this? |
pugnascotia
commented
Feb 3, 2017
@gaearon Any chance of this getting merged? |
alex-at-oclc
commented
Feb 3, 2017
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 I'd prefer to add 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 #172, but managed to get it working all in a single WebpackDevServer in the same process. I added a 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. |
pugnascotia
commented
Feb 4, 2017
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. |
gaearon
commented
Feb 4, 2017
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! |
pugnascotia
commented
Feb 4, 2017
OK, thanks for the update. |
23a9f98 to
44fb8afComparepugnascotia
commented
Feb 4, 2017
I've rebased off master and tidied up a couple things, no biggies. |
alex-at-oclc
commented
Feb 6, 2017
@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. |
ada13a6 to
79a7e0aCompareDerive a library name from the project name in package.json. In the index.js template, don't render the App unless window is defined, and export a render-to-string function.
79a7e0a to
aedc2b1Comparepugnascotia
commented
Mar 22, 2017
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! |
Timer
commented
Mar 22, 2017
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. |
pugnascotia
commented
Mar 23, 2017
I'm up for changing that, can we thrash what we need? How about:
Exclusions:
|
Start implementing intelligent server bundling by parsing the app's entrypoint and looking for an exported function.
gaearon
commented
May 8, 2017
I’m growing hesitant about this, though I really appreciate the effort that went into it. There are a few things that concern me:
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? |
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. 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 |
gaearon
commented
May 8, 2017
Sounds good.
Is there a downside to a separate file like |
Timer
commented
May 8, 2017
There's nothing wrong with this other than requiring some (possibly) duplicated code; it's probably a bit more explicit (which is better). |
pugnascotia
commented
May 9, 2017
So would the idea with For dynamic imports, I've been experimenting with defining an extra CRA Babel package that pulls in |
Yup, that's the idea! This removes some of that implicit parsing magic (I'm really sorry you spent the time working on it, but we might use it in the future).
I'm not very experienced with SSR so maybe you can help shed some light on this. Out of interest in your time @pugnascotia, please don't move forward with any changes yet (unless you really want to). I don't want you to waste your time until we figure out the specifics (in case we change our minds). 😄 |
pugnascotia
commented
May 9, 2017
It's a fair point, but Node still doesn't support ES6 modules yet, so some kind of compilation would still required. Same if you're using experimental ES features, or minifying for performance reasons, etc. |
pugnascotia
commented
May 9, 2017
(BTW, my original reason for getting involved in this was SSR using Nashorn in a Java / Spring app. So, you know, node-what? :-)) |
We'd just run it against our |
cr101
commented
May 16, 2017
I'm a React beginner and I believe that building a React app with server-side rendering is essential simply because it just makes sense. |
raymondsze
commented
May 21, 2017
@pugnascotia |
pugnascotia
commented
Jul 5, 2017
My original intent with this PR was to make it possible to consume a CRA app as a library / module, so that I could performing server rendering in a Java application using the Nashorn JS engine. @gaearon makes a good point that other projects are tackling SSR as a primary concern - given that at the moment there's no coherent story in CRA around SSR, and that we could add value right now in a discrete way by exporting the app as a UMD module, what do we think about adding the export and coming back to SSR later? |
In order to facilitate server-side rendering (#1100), 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.