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

[RFC] Use SSG for frontend #824

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft

[RFC] Use SSG for frontend #824

wants to merge 6 commits into from

Conversation

jameshadfield
Copy link
Member

@jameshadfield jameshadfield commented Apr 11, 2024

CSR, SSG, SSG, ...

This section is mostly background

The current site is entirely using client side rendering (CSR) - we send the client a minimal HTML page which then loads JS which then paints the page. See #810 for more details.

Pre-rendering the page as HTML (or as much of the page reasonably possible) at build time is typically called static-site-generation (SSG), but it's also referred to as a special case of server-side-rendering (SSR). Next.js mostly refers to it as SSR, but here i'll use SSG to differentiate from on-request SSR. The client still receives JS (essentially the same as in CSR) and once that's loaded that ~replaces the initial HTML ("hydration").

Per-request SSR (often just called SSR) is similar but the rendering to HTML happens on the server for each request. The HTML is sent to the client and hydrated. In theory you can cache a previous rendered version and use that as required, but it's unclear how much control/clarity Next.js provides here.

Both have advantages in certain situations such as improved SEO, faster time-to-first-paint, ability to work without JS. There are disadvantages which I'll expand on below. None of these advantages are all that compelling to me in the context of Nextstrain, but if you could have them for free then I'd take them. We used SSG with Gatsby, but it was broken (and no-one noticed?); see below for more on this.

Note: None of this is related to React Server Components (RSC). Next.js supports them. We don't use them. We can discuss elsewhere if people want to use them. RSC needs SSR, but SSR doesn't need RSC.

This PR

Implements SSG for 3 pages: the splash page, the /pathogens page and the /team page. One per-request SSR page is included, /example-ssr-on-request. The code is proof-of-principle, largely to explore whether we should be heading in this direction.

To test changes you probably want to run with JS disabled or by throttling the connection to a slow speed so you can see any changes as the page hydrates. Logging in is useful as you will see the login button flash. Update: Can't login on review apps! For the time being you can compare with next.nextstrain.org (Next.js but entirely CSR) or nextstrain.org (Gatsby).

Testing changes is easiest by running the server locally and comparing pages modified in this PR with those that aren't. Gatsby is no longer in use on any of our servers, but you could use a worktree to see a side-by-side comparison if desired.

Costs

The main costs in my eyes are the complexity / increased barrier to entry for developers who now need to be aware that the code will run both on the server and on the client. #810 was mainly motivated by reducing development cost, and I'm hesitant to add another layer we have to be cognisant of. If the rendering is different (server vs client) then it often results in a nasty flash as the page hydrates and the layout changes. Next.js (in dev mode) seems to highlight most hydration errors which is nice. A very similar and related case is ubiquitous on the web where the "login" icon will be shown initially and then it'll change to your name, behaviour which exists in this PR too. The Perils of Hydration goes into more detail and has many examples.

There are also other costs for SSR beyond those for SSG such as increased server load and less isolation between the server and client.

We were using SSG on Gatsby, right?

Yes. But it was rarely the same as the hydrated version. I don't think we really noticed? Below shows a series of screenshots, the far left is the Gatsby page with JS disabled (i.e. before it's hydrated), the middle is the Next.js page before it's hydrated (running from this PR), and the right page is the hydrated page (Next.js, but Gatsby should be identical).

image
Main splash page. Gatsby SSG is missing everything beyond the header.

image

/team worked well on Gatsby

image

/pathogens. SSG shows us the (animated) spinner because the resource listing data is fetched client-side by an API request. Note that there was a subtle bug in Gatsby where the SSG version didn't load the avatars in the footer and they'd flash in when hydrated; not a huge deal but an example of how these things happened and we didn't realise. This bug was flagged up by Next.js and fixed in this PR by 3cfb239. Finally, If we used SSR we could skip the API request and load the page with the listing shown, but at a cost.

What to do?

I'm pleasantly surprised how nice the Next.js SSG experience is. I'm inclined to use it, but wanted to gauge opinions from others first. Beyond fixing hydration bugs, switching from CSR to SSG is a trivial change in each file within ./static-site/pages:

- import dynamic from 'next/dynamic'
- const Index = dynamic(() => import("../src/pages/index"), {ssr: false})
+ import Index from "../src/pages/index";

I don't think SSR is the right move, but I included a commit here to show how we could do it (c98760c is a more thorough way to share data between the node processes). A nice aspect of next.js' design is that we could use SSR for a select set of pages (e.g. /whoami) if we really wanted to, and even do so whilst maintaining the static-site/pages/* based routing.

The page will be generated using Next.js SSR (only compiled once, at
build time, not when the server is running despite the first _S_ in SSR
standing for Server. At least, that's my understanding, but I need to
confirm this.)

Notice that _without_ JS (i.e. the HTML that's delivered to the browser)
the layout is completely wrong. With JS enabled you'll get a flash as
the browser hydrates and client side rendering updates the DOM to use
the correct layout. The layout difference is due to the styled
components styles not being included in CSS.
the unhydrated site looks pretty good but there are hydration errors
raised by next.js (only in dev mode)
The underlying hydration errors are caused by different code paths on
the client vs server, namely using a conditional querying `window`.
@nextstrain-bot nextstrain-bot temporarily deployed to nextstrain-s-james-ssr-4i28wwa April 11, 2024 03:18 Inactive
@jameshadfield jameshadfield requested a review from victorlin April 11, 2024 03:18
@jameshadfield jameshadfield changed the title [WIP] Use SSG for frontend [RFC] Use SSG for frontend Apr 11, 2024
@@ -1,3 +1,8 @@
/**
*
* Example of dynamic import but using ssr (which is the default dynamic import!)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It took me a while to work out the difference between a dynamic import as used here and a typical import as used in the /team page. TL;DR we should avoid dynamic imports and just use normal imports.

In terms of build-time rendered / server-rendered HTML there is no difference.

The difference comes client-side, where the next.js docs explain:

By using next/dynamic, the [dynamically imported] component will not be included in the page's initial JavaScript bundle. The page will render the Suspense fallback first, followed by the [dynamically imported] component when the Suspense boundary is resolved.

So when we use a <Link> to perform a client-side navigation to the splash page we will initially get the fallback which is then replaced by the contents. So it's better to avoid these entirely and just use normal imports. If you want to try this out it helps to add in a loading: () => (<p>Loading...</p>) option to the dynamic import. (The reason I used dynamic imports in the first place is to force the site into CSR mode.)

It took me a while to understand this because I thought next.js was using the approach described in New Suspense SSR Architecture in React 18 where suspended components will / can be replaced by their fallback component in the streamed HTML, and then a later part of the HTML will include the contents of the component + a small script to tell the browser to replace the fallback. But that's not in play here.

@nextstrain-bot nextstrain-bot temporarily deployed to nextstrain-s-james-ssr-4i28wwa April 12, 2024 02:51 Inactive
@tsibley tsibley temporarily deployed to nextstrain-s-james-ssr-avldwwo May 10, 2024 05:44 Inactive
@tsibley
Copy link
Member

tsibley commented May 10, 2024

Thanks for this prototyping, @jameshadfield.

I think at the very least we should be doing some form of server-side rendering: either ahead-of-time (SSG) or on-request (traditional SSR) or both. The visitor experience just seems nicer (faster page loads, less initial render replaced by a big update a second later) at nearly no development cost. For some pages, I continue to think on-request SSR (over CSR) would make both our development experience nicer (avoid APIs we don't need) and the visitor experience (immediate paint, not delayed by further API requests).

I'm curious, why did you choose to stick with the older Next.js "pages router" instead of it's newer "app router"? I'd generally think to start with the latter…

@jameshadfield
Copy link
Member Author

Thanks for responding!

I think at the very least we should be doing some form of server-side rendering: either ahead-of-time (SSG) or on-request (traditional SSR) or both.

SSG is (as this PR shows) straightforward and the current design of the site wouldn't immediately benefit from SSR so it seems there's agreement to do the small amount of work necessary to have SSG in the short term.

For some pages, I continue to think on-request SSR (over CSR) would make both our development experience nicer (avoid APIs we don't need) and the visitor experience (immediate paint, not delayed by further API requests).

Tom & I talked about this a week or so ago. Under the current design where all data is obtained through APIs then I still don't think SSR is worth it. Pros and cons to this design. Under a different design where you replace APIs with accessing the data at render time on the server side then of course SSR is needed (by definition). Trade offs.

I'm curious, why did you choose to stick with the older Next.js "pages router" instead of it's newer "app router"? I'd generally think to start with the latter…

I felt it was much closer in design to the Gatsby design and wanted to make a PR with as little changes as possible (both conceptual and number of lines). Nothing stopping future PR switching, either all at once or gradually.

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