Investigate support for Universal/Isomorphic rendering #27
Comments
|
This is exciting! The first step would be to take https://github.com/insin/react-hn/blob/master/src/index.js and produce a server variant that could run via something like Express. I've no idea how that would play with SWs but I'd be interested to find out! |
|
I'm happy to take a stab at it btw if you'd like me to. |
|
@jackfranklin That would be really appreciated. Would love to see how you think this should be structured. |
|
@addyosmani so I started playing around with this and hit the fact that all the stores that are used here reference The other thing to do is polyfill it in with JSDOM or something but I'm not sure I'm too keen on that idea. LMK what you think. |
|
So for now I wrapped every This seems related to facebook/react#6232, but I haven't had time to dig into it yet to figure out what the issue is. If you want to take my (incredibly hacky, not proper code) and play with it, it's on this branch: https://github.com/jackfranklin/react-hn/tree/universal-experiment. |
|
@jackfranklin Thanks a lot for exploring this so quickly! I'll give your branch a whirl. From the looks of that issue, the exception you're seeing may be specific to certain versions of browsers being tested in (but I think it may actually be a React bug). What were you seeing this bug in?
I think that if we need to rethink a little of our architecture for the content side, it would be awesome to at least be able to deliver the app shell without data from the server as a very first step. Especially if it isn't gated on the issue you're seeing after wrapping those refs :) |
|
@addyosmani I'm seeing that error on the Node server, not on the browser. The server returns back some HTML (which looks fine) but then crashes with that error. Not entirely sure what's going on there, although I ran out of time to properly try to debug it. I hopefully can take a look later. Agree that 1st step is delivering the shell, I don't see any reason why that isn't possible with the refs wrapped :) |
|
@jackfranklin It looks like that error you were running into is reproducible with React 15. I haven't been able to find a workaround that worked for me in facebook/react#6232, but I might also be missing something totally obvious. Reading through facebook/react#3298 too in case we're having related issues. |
|
@jackfranklin I got your branch working by changing Or more specifically probably in firebase-node, didn't get very far into that though |
|
@psimyn Were you able to get the shell fully rendering in browser when you switched to componentWillMount? We do currently use sessionStorage and localStorage for caching history positions and a very high level set of item cache indices, but I think it's possible to work around this. |
|
Seems to all work fine - nav/paging links all work (loading page with JS disabled), and posts show loading dots
I think protection around the cache setting is needed - |
|
I think you're right. Anyone interested in playing with shielding caching as a next step here? I've been trying to play with/figure out what the major blockers for server rendering the main content (topstories, comments) are. Beyond shields, we currently use the main real-time Firebase API for content. I wonder if we would need to switch to (or fallback) to their REST API and use node-fetch + just JSON responses to get the data working as expected. I'm also looking at https://github.com/jsdf/hacker-news-mobile, https://github.com/camsong/redux-hacker-news and https://github.com/moretti/react-isomorphic-hn for pointers. (Side: if we do get isomorphic rendering working it will help a lot with the offline support I've been exploring) |
|
For anyone interested: https://github.com/insin/react-hn/tree/isomorphic-take1 includes @jackfranklin's work, some further tweaks I made to bring back hostname parsing without using the DOM and one or two of the fixes mentioned in this thread to fix breakage. |
|
So we've now landed universal rendering for the UI shell. We don't yet do this for the content. As part of introducing 'Offline Mode' (in Settings) we landed support for a REST-based HN Service over in https://github.com/insin/react-hn/blob/master/src/services/HNServiceRest.js. @jackfranklin suggested that we could maybe use https://github.com/ericclemmons/react-resolver as way to get universal rendering working for us, maybe using the REST service. Perhaps we could expose this initially as a Setting that would let you switch on full Universal rendering support? i.e if on, use resolver + REST instead of live updates. |
|
I suggest react-resolver only because I've used it once on a small demo and it worked really nicely - happy to take other ideas if anyone else has more experience. @addyosmani how are you setting that flag ? Are you thinking query param? So default is effectively what we have now, and you go for |
|
@jackfranklin Thanks again for any help here! There are a few ways we could tackle this. After discussing it a little further on Twitter, a few ideas that dropped out:
|
|
I started roughly playing with app.get('*', function(req, res) {
ReactRouter.match({
routes: routes,
location: req.url
}, function(err, redirectLocation, renderProps) {
if (err) {
res.status(500).send(err.message)
}
else if (redirectLocation) {
res.redirect(302, redirectLocation.pathname + redirectLocation.search)
}
else if (renderProps) {
Resolver
.resolve(function() {return RouterContext(renderProps)})
.then(function(resolverRes) {
var markup = renderToString(
React.createElement(resolverRes.Resolved, renderProps, null)
)
res.render('index', {
markup: markup,
scriptTag: '<script>window.__REACT_RESOLVER_PAYLOAD__ = ' + JSON.stringify(resolverRes.data) + '</script>'
})
})
}
else {
res.sendStatus(404)
}
})
})which I think looks okay. Note that after the next sets of changes I still don't manage to get I then updated the app's var Resolver = require('react-resolver').Resolver
Resolver.render(function() { return <Router history={history} routes={routes}/> }, document.getElementById('app'))I then started looking at what needed to be done to switch one component (Comments) to use resolver for resolving our data. Most components requiring firebase have a module.exports = resolve('comment', function(props) {
return HNServiceRest.itemRef(props.id).then(function(res) {
return res.json()
}).then(function(snapshot) {
return snapshot
})
})(Comment)Client-side, snapshots get fetched correctly here (at least, we can log them to console) but nothing appears to happen server-side. We're also no longer using replaceState, which was previously relied on to get the view updated correctly. This might be part of what I'm missing. I'll try taking another look at this in a few days. |
|
Takeaway from my Until then, I started looking into just using Basically, something like: statics: {
fetchData: function(props) {
return HNServiceRest.itemRef(props.id).then(function(res) {
return res.json()
})
}
},We would then need to schedule these fetches from our server-side before rendering. Something like this. |
|
Updating this thread with progress on local experiments:
Unfortunately, because of the way the official Firebase HN API is setup, there are quite a lot of network requests that need to be resolved (fetch index of stories, then fetch each story in order to display 'top stories') before we can return anything useful from the server for first render. This negates the benefit of SSR in our case because a user ends up waiting 30-60s for content to get returned from the 'local' server. We can imagine similar problems being experienced with the comments page. We could work around this by maintaining our own cache that's periodically refreshed and perform an SSR render from there. I'm going to spend a little time rethinking how we can tackle this problem. The bottleneck atm is less so "can we SSR our content with React" and more so "how do we do this efficiently on the server with caching". In case folks are wondering, others seem to have tried solving this by setting up their own unofficial HN APIs: https://github.com/cheeaun/node-hnapi, http://hn.algolia.com/api. I might build a very lightweight server-rendered experience for the first view using this and then 'progressively upgrade' to what we have now. |
In the application shell architecture, we try to encourage server-side rendering for fast first paint as much as possible. Even if just for the "shell"/common user interface.
In our current version of react-hn in master, we still rely on client-side rendering to display any content. We can probably do way better here so a static render gives us something useful on the screen.
I would love to see what we can do to explore Universal rendering support and what perf gains this would bring. We would want this to play as nicely as possible with our Service Worker caching but..good to get a start on a PR to see what a V0 take on this would look like.
cc @jackfranklin @Garbee @zenorocha who may be interested in helping with this or have a few pointers 🍰
The text was updated successfully, but these errors were encountered: