Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for server-side rendering Top Stories & Comments #47

Merged
merged 8 commits into from
Jun 23, 2016

Conversation

addyosmani
Copy link
Collaborator

@addyosmani addyosmani commented Jun 9, 2016

Please don't merge in just yet. This work requires some discussion about the UX

This set of changes adds server-side rendering for both Top Stories (index) and comment threads. It enables React HN to deliver content quickly, especially useful for flakey/slow networks or where our vendor/app bundle hasn't been fully loaded yet (sometimes a few seconds on slow connections). It works with JavaScript disabled too.

Demo: https://react-hn-ssr.appspot.com
Video: https://www.youtube.com/watch?v=E1YNRc3Kfag&feature=youtu.be
Lighthouse scores: 100/100

How

I investigated a few different ways SSR could be added over in #27. The approach I landed on moves most of the work over into Express, with minimal changes to our React code to support 'preboot' data and hydration. We populate the view in componentWillMount (server/client) and hydrate in componentDidMount (client).

Originally, I tried to do full server-side rendering using the Firebase SDK. This involved a large number of network requests to be resolved for stories and comments, which ended up taking anywhere up to a minute. In a normal server environment, we would start adding in a caching layer here to avoid each visit having to incur this cost.

Thankfully, there exists a third-party (unofficial) Hacker News API we can call, which does support caching and provides story and comment data in a slightly more friendly format. Because the data format is different enough to what we currently rely on in the rest of our React components (in particular, our templates), I decided to replicate the output in EJS templates allowing us to get the same look and feel once output.

Some screenshots from WebPageTest

First meaningful paint happening at 0.6s on cable with SSR compared to 2.1s with our current build:

screen shot 2016-06-09 at 14 30 41

Hydration to real-time content, occurring at roughly the same time as our current build:

screen shot 2016-06-09 at 14 32 15

http://www.webpagetest.org/video/compare.php?tests=160609_TS_1H8W,160609_SD_1H8Y if you'd like to look at the data directly.

UX

Perhaps the biggest change this PR adds is a change to the ReactHN UX. If you navigate to the demo with JS off/on a throttled connection, content will load/display quickly and eventually 'update' to real-time. On a fast connection, you'll see the server-rendered content and then very shortly after the hydrated real-time experience. It's a little like FOUC. This only happens for the first page of top story results (after which the experience is per usual), however I'm interested in hearing what others think about the experience. Is it okay? Does it feel unnatural? If the latter, there are a few options we have, such as delaying hydration until a future time (based on an interval). Alternative ideas are also welcome.

The official Firebase API (https://github.com/HackerNews/API) requires
multiple network
connections to be made in order to fetch the list of Top Stories
(indices) and then the
summary content of these stories. Directly requesting these resources
makes server-side
rendering cumbersome as it is slow and ultimately requires that you
maintain your own
cache to ensure full server renders are efficient.

To work around this problem, we can use one of the unofficial Hacker
News APIs, specifically
https://github.com/cheeaun/node-hnapi which directly returns the Top
Stories and can cache
responses for 10 minutes. In ReactHN, we can use the unofficial API for
a static server-side
render and then 'hydrate' this once our components have mounted to
display the real-time
experience.

The benefit of this is clients loading up the app that are on flakey
networks (or lie-fi)
can still get a fast render of content before the rest of our
JavaScript bundle is loaded.
@addyosmani
Copy link
Collaborator Author

This PR also needs to be updated once #46 lands.

@insin
Copy link
Owner

insin commented Jun 10, 2016

The flash is a bit jarring, but the performance stats can't be argued with 😸 I wonder if there's something we could do with transitions to make the jump from static to live a little less jarring, and even in general? The UI also jumps about when the order of things changes in the API, which can be annoying in fresh, active threads.

I would stil like to investigate maintaining our own cache from the API at some stage, keeping the story lists fresh and just grabbing the story and top few comments for thread which aren't in the cache. Finding out where my spare time lives again, but I've been spending my open source points on nwb lately.

@addyosmani
Copy link
Collaborator Author

addyosmani commented Jun 11, 2016

@insin I like the idea of a transition. I'll try that out. One other experiment I'm playing with (might still be jarring) is dropping the loading spinners for just the first page of results where SSR is used. It looks a little like this: http://recordit.co/q3NqUpVghE

@insin
Copy link
Owner

insin commented Jun 12, 2016

I'm playing with using the live Firebase API on the server to keep the top X pages of every list section in memory, including their child contents (comments and poll items). This would not only give us an accurate initial render, but we could also fetch content for the first few pages of stories (e.g. load the first X comments which will appear regardless of nesting so the user gets a full screen of content while the rest loads, or that plus all the top-level comments)

@addyosmani
Copy link
Collaborator Author

@insin An accurate initial render would be so good to have here. I also like the idea of prefetching the first few pages in advance. I'm curious to find out what our caching strategy (for the server) might look like as I think some of the staleness issues the third-party used here have are around updating in-memory versions every 10m. Very interested to see where your experiments go :)

@insin
Copy link
Owner

insin commented Jun 12, 2016

This is what I'm playing with: https://gist.github.com/insin/13d8a150cb7d9c2938c3c45f7927fb11

@addyosmani
Copy link
Collaborator Author

@insin Took a look through your experiment on in-memory caching. Looks neat. I was curious if you saw this work as being short-term vs long-term and whether you felt there was value in me continuing to finish up this PR before the in-memory work is ready. Totally fine with whichever you prefer :)

@addyosmani
Copy link
Collaborator Author

I've updated the implementation to only display the change in UX for the first page of top story results. As soon as the ReactHN React bundle gets loaded and we switch to client-side routing, everything works the same as it did before. Video: http://recordit.co/vPho97mZCn

@addyosmani
Copy link
Collaborator Author

After talking it over with @insin on DM, this seems fine for us to merge now and improve later as needed. Landing ships ahoy! ⚓ 🚢

@addyosmani addyosmani merged commit f05849d into master Jun 23, 2016
@addyosmani addyosmani deleted the server-render branch June 23, 2016 11:28
@insin
Copy link
Owner

insin commented Jun 23, 2016

:shipit: 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants