Investigate support for Universal/Isomorphic rendering about react-hn HOT 19 CLOSED

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 - onStoriesUpdated is where it seems to break when using componentWillMount

from react-hn.

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!

from react-hn.

I'm happy to take a stab at it btw if you'd like me to.

from react-hn.

@jackfranklin That would be really appreciated. Would love to see how you think this should be structured.

from react-hn.

@addyosmani so I started playing around with this and hit the fact that all the stores that are used here reference window a lot for local storage and for binding to events. I'm still trying to get my head around all the code here but have you any thoughts on this? Maybe we can just avoid using the stores all together on the server and just render the app shell with no data from the server? That would be a good first step at least?

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.

from react-hn.

So for now I wrapped every window or document reference in an if typeof ... check, just to see if we can get it going. When running the server though the first server side render does sort of work, but then I get:

Invariant Violation: React DOM tree root should always have a node reference.
    at invariant (/Users/jackfranklin/git/react-hn/node_modules/fbjs/lib/invariant.js:38:15)
    at getNodeFromInstance (/Users/jackfranklin/git/react-hn/node_modules/react/lib/ReactDOMComponentTree.js:164:67)
    at ReactDOMComponent.Mixin._updateDOMProperties (/Users/jackfranklin/git/react-hn/node_modules/react/lib/ReactDOMComponent.js:786:20)
    at ReactDOMComponent.Mixin.updateComponent (/Users/jackfranklin/git/react-hn/node_modules/react/lib/ReactDOMComponent.js:686:10)
    at ReactDOMComponent.Mixin.receiveComponent (/Users/jackfranklin/git/react-hn/node_modules/react/lib/ReactDOMComponent.js:643:10)
    at ReactDOMComponent.wrapper [as receiveComponent] (/Users/jackfranklin/git/react-hn/node_modules/react/lib/ReactPerf.js:66:21)
    at Object.ReactReconciler.receiveComponent (/Users/jackfranklin/git/react-hn/node_modules/react/lib/ReactReconciler.js:103:22)
    at [object Object].ReactCompositeComponentMixin._updateRenderedComponent (/Users/jackfranklin/git/react-hn/node_modules/react/lib/ReactCompositeComponent.js:653:23)
    at [object Object].ReactCompositeComponentMixin._performComponentUpdate (/Users/jackfranklin/git/react-hn/node_modules/react/lib/ReactCompositeComponent.js:635:10)
    at [object Object].ReactCompositeComponentMixin.updateComponent (/Users/jackfranklin/git/react-hn/node_modules/react/lib/ReactCompositeComponent.js:564:12)

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.

from react-hn.

@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?

Maybe we can just avoid using the stores all together on the server and just render the app shell with no data from the server? That would be a good first step at least?

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 :)

from react-hn.

@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 :)

from react-hn.

@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.

Invariant Violation: React DOM tree root should always have a node reference.
    at invariant (/Users/addyo/projects/react-hn-universal-experiment/node_modules/fbjs/lib/invariant.js:38:15)
    at getNodeFromInstance (/Users/addyo/projects/react-hn-universal-experiment/node_modules/react/lib/ReactDOMComponentTree.js:164:67)
    at ReactDOMComponent.Mixin._updateDOMProperties (/Users/addyo/projects/react-hn-universal-experiment/node_modules/react/lib/ReactDOMComponent.js:786:20)
    at ReactDOMComponent.Mixin.updateComponent (/Users/addyo/projects/react-hn-universal-experiment/node_modules/react/lib/ReactDOMComponent.js:686:10)
    at ReactDOMComponent.Mixin.receiveComponent (/Users/addyo/projects/react-hn-universal-experiment/node_modules/react/lib/ReactDOMComponent.js:643:10)
    at ReactDOMComponent.wrapper [as receiveComponent] (/Users/addyo/projects/react-hn-universal-experiment/node_modules/react/lib/ReactPerf.js:66:21)
    at Object.ReactReconciler.receiveComponent (/Users/addyo/projects/react-hn-universal-experiment/node_modules/react/lib/ReactReconciler.js:103:22)
    at [object Object].ReactCompositeComponentMixin._updateRenderedComponent (/Users/addyo/projects/react-hn-universal-experiment/node_modules/react/lib/ReactCompositeComponent.js:653:23)
    at [object Object].ReactCompositeComponentMixin._performComponentUpdate (/Users/addyo/projects/react-hn-universal-experiment/node_modules/react/lib/ReactCompositeComponent.js:635:10)
    at [object Object].ReactCompositeComponentMixin.updateComponent (/Users/addyo/projects/react-hn-universal-experiment/node_modules/react/lib/ReactCompositeComponent.js:564:12)
addyo at addyo-macbookpro in ~/projects/react-hn-universal-experiment
$ node src/server.js
Listening at http://localhost:3003
FIREBASE WARNING: Exception was thrown by user callback. Invariant Violation: React DOM tree root should always have a node reference.
    at invariant (/Users/addyo/projects/react-hn-universal-experiment/node_modules/fbjs/lib/invariant.js:38:15)
    at getNodeFromInstance (/Users/addyo/projects/react-hn-universal-experiment/node_modules/react/lib/ReactDOMComponentTree.js:164:67)
    at ReactDOMComponent.Mixin._updateDOMProperties (/Users/addyo/projects/react-hn-universal-experiment/node_modules/react/lib/ReactDOMComponent.js:786:20)
    at ReactDOMComponent.Mixin.updateComponent (/Users/addyo/projects/react-hn-universal-experiment/node_modules/react/lib/ReactDOMComponent.js:686:10)
    at ReactDOMComponent.Mixin.receiveComponent (/Users/addyo/projects/react-hn-universal-experiment/node_modules/react/lib/ReactDOMComponent.js:643:10)
    at ReactDOMComponent.wrapper [as receiveComponent] (/Users/addyo/projects/react-hn-universal-experiment/node_modules/react/lib/ReactPerf.js:66:21)
    at Object.ReactReconciler.receiveComponent (/Users/addyo/projects/react-hn-universal-experiment/node_modules/react/lib/ReactReconciler.js:103:22)
    at [object Object].ReactCompositeComponentMixin._updateRenderedComponent (/Users/addyo/projects/react-hn-universal-experiment/node_modules/react/lib/ReactCompositeComponent.js:653:23)
    at [object Object].ReactCompositeComponentMixin._performComponentUpdate (/Users/addyo/projects/react-hn-universal-experiment/node_modules/react/lib/ReactCompositeComponent.js:635:10)
    at [object Object].ReactCompositeComponentMixin.updateComponent (/Users/addyo/projects/react-hn-universal-experiment/node_modules/react/lib/ReactCompositeComponent.js:564:12)
/Users/addyo/projects/react-hn-universal-experiment/node_modules/firebase/lib/firebase-node.js:52
(d="0"+d),c+=d;return c.toLowerCase()}var zd=/^-?\d{1,10}$/;function td(a){return zd.test(a)&&(a=Number(a),-2147483648<=a&&2147483647>=a)?a:null}function ec(a){try{a()}catch(b){setTimeout(function(){R("Exception was thrown by user callback.",b.stack||"");throw b;},Math.floor(0))}}function S(a,b){if(t(a)){var c=Array.prototype.slice.call(arguments,1).slice();ec(function(){a.apply(null,c)})}};function Ad(a){var b={},c={},d={},e="";try{var f=a.split("."),b=Pb(id(f[0])||""),c=Pb(id(f[1])||""),e=f[2],d=c.d||{};delete c.d}catch(g){}return{ph:b,Dc:c,data:d,bh:e}}function Bd(a){a=Ad(a).Dc;return"object"===typeof a&&a.hasOwnProperty("iat")?z(a,"iat"):null}function Cd(a){a=Ad(a);var b=a.Dc;return!!a.bh&&!!b&&"object"===typeof b&&b.hasOwnProperty("iat")};function Dd(a){this.Y=a;this.g=a.n.g}function Ed(a,b,c,d){var e=[],f=[];Na(b,function(b){"child_changed"===b.type&&a.g.Ad(b.Le,b.Ma)&&f.push(new H("child_moved",b.Ma,b.Ya

Invariant Violation: React DOM tree root should always have a node reference.
    at invariant (/Users/addyo/projects/react-hn-universal-experiment/node_modules/fbjs/lib/invariant.js:38:15)
    at getNodeFromInstance (/Users/addyo/projects/react-hn-universal-experiment/node_modules/react/lib/ReactDOMComponentTree.js:164:67)
    at ReactDOMComponent.Mixin._updateDOMProperties (/Users/addyo/projects/react-hn-universal-experiment/node_modules/react/lib/ReactDOMComponent.js:786:20)
    at ReactDOMComponent.Mixin.updateComponent (/Users/addyo/projects/react-hn-universal-experiment/node_modules/react/lib/ReactDOMComponent.js:686:10)
    at ReactDOMComponent.Mixin.receiveComponent (/Users/addyo/projects/react-hn-universal-experiment/node_modules/react/lib/ReactDOMComponent.js:643:10)
    at ReactDOMComponent.wrapper [as receiveComponent] (/Users/addyo/projects/react-hn-universal-experiment/node_modules/react/lib/ReactPerf.js:66:21)
    at Object.ReactReconciler.receiveComponent (/Users/addyo/projects/react-hn-universal-experiment/node_modules/react/lib/ReactReconciler.js:103:22)
    at [object Object].ReactCompositeComponentMixin._updateRenderedComponent (/Users/addyo/projects/react-hn-universal-experiment/node_modules/react/lib/ReactCompositeComponent.js:653:23)
    at [object Object].ReactCompositeComponentMixin._performComponentUpdate (/Users/addyo/projects/react-hn-universal-experiment/node_modules/react/lib/ReactCompositeComponent.js:635:10)
    at [object Object].ReactCompositeComponentMixin.updateComponent (/Users/addyo/projects/react-hn-universal-experiment/node_modules/react/lib/ReactCompositeComponent.js:564:12)

Reading through facebook/react#3298 too in case we're having related issues.

from react-hn.

@jackfranklin I got your branch working by changing componentWillMount to componentDidMount in Stories component. Haven't gone through it to find what is breaking on server, but I suspect something in StoryStore.

Or more specifically probably in firebase-node, didn't get very far into that though

from react-hn.

@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.

from react-hn.

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)

from react-hn.

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.

from react-hn.

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.

from react-hn.

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 ?rest=1 or something that would get you REST API + server resolved data and no/some live updates on the client?

from react-hn.

@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:

• By default, use a server-side/universal render (with REST + react-resolver) for the first view of any page. Once JS has loaded, switch to the normal HNService.js adapter (Firebase/WebSockets) for any real-time updates. This avoids needing to introduce any flags or query params
• Consider react-resolver an experiment here. We would trigger it by passing a query-param which would give you REST API + server resolved data and (like above) once JS is fully loaded try to switch to the HNService normal client for updates. A first pass could ignore trying to do the switch.

from react-hn.

I started roughly playing with react-resolver over the weekend and run into a few brick walls (likely due to my own ignorance). The server.js changes I made look a little like this:

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 resolverRest.data populated with anything

I then updated the app's index.js to use resolver for rendering instead of the straight-up ReactDOM.render directly:

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 bindFirebaseRef method called during componentWillMount. Rather than calling this, I took inspiration from isomorphic-demo by @allenkim67 and switched this over to doing the following in the module.exports of that component:

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.

from react-hn.

Takeaway from my react-resolver experiments: didn't quite work out well for our needs. If someone wants to take a fresh stab at it, more than welcome :)

Until then, I started looking into just using statics inside our components to define a fetchData routine that could be called from either the client or the server. I've seen a few different universal React apps do this and I think it might help us reach our end goal for content rendering.

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.

from react-hn.

Updating this thread with progress on local experiments:

• Tried fetchData statics for each component that we Promise.all resolve before server rendering
• Tried "Fat" router that fetches the top stories then resolves the child entries before sending back to props

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.

from react-hn.