Adds isomorphic/server-side React application-shell rendering #35
Conversation
This is necessary for a successful deploy to either Heroku or Google App Engine given we now need all dependencies defined upfront. I’m also enforcing Node 6.x as earlier versions now timeout thanks to phantomjs.
Enables deployments
Introduces `deploy`, localises a lot of the current binaries so that they don’t require global installs (and work fine on Heroku and AppEngine). `serve` now gives you the old `start` behaviour and `start` now just runs the node server that we default to.
|
I am not really React-guy, but if no one will be able to review it, then I'll do review. |
|
Thanks! Any help at all with reviews would be 💕 |
|
Awesome - I'll take a look through quickly if that's useful :) |
|
@jackfranklin That would be super useful :) @insin would you like for us to wait on your final call after other reviews are done before merging this in if we get go aheads? |
| @@ -35,7 +35,7 @@ var Stories = React.createClass({ | |||
| } | |||
| }, | |||
|
|
|||
| componentWillMount() { | |||
| componentDidMount() { | |||
There was a problem hiding this comment.
more out of interest, but why is this required? Is this what gets around that weird React error I was getting?
There was a problem hiding this comment.
componentDidMount() only gets called on the client
|
Looks great! |
| @@ -0,0 +1,5 @@ | |||
| runtime: nodejs | |||
There was a problem hiding this comment.
Does this come with any predefined environment variables?
If not, setting NODE_ENV to production will make Express cache the view template
There was a problem hiding this comment.
You're right. It looks like GAE does support environment variables: https://cloud.google.com/appengine/docs/flexible/nodejs/configuring-your-app-with-app-yaml#Node.js_app_yaml_Defining_environment_variables. I'll update.
|
You could use something like this |
| "lint:fix": "eslint --fix .", | ||
| "start": "nwb serve", | ||
| "precache": "sw-precache --root=public --config=sw-precache-config.json" | ||
| "build": "npm run lint && cp node_modules/sw-toolbox/sw-toolbox.js public/sw-toolbox.js && ./node_modules/nwb/lib/bin/nwb.js build && npm run precache", |
There was a problem hiding this comment.
can replace ./node_modules/nwb/lib/bin/nwb.js with ./node_modules/.bin/nwb
There was a problem hiding this comment.
Good catch. Will update.
|
LGTM! |
Demo URL: https://react-hn.appspot.com
tl;dr: we can now server-side render the UI, which also enables it to be visible with or without JavaScript enabled
This set of changes formalises the work done in #27 (huge thanks to @jackfranklin and @psimyn) for helping us get it started. The end product can now be trivially deployed to Google App Engine.
@jackfranklin: some of the specific changes I added to your branch were bringing back SSR-friendly hostname parsing, the
componentDidMountfix, slightly tighter equality checks and updates to the build pipeline which enable us to deploy to GAE.cc @insin @NekR