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

Ensure static pages get prerendered when SSR is disabled #11639

Open
benmccann opened this issue Jan 15, 2024 · 6 comments
Open

Ensure static pages get prerendered when SSR is disabled #11639

benmccann opened this issue Jan 15, 2024 · 6 comments

Comments

@benmccann
Copy link
Member

benmccann commented Jan 15, 2024

Describe the problem

People enable SPA without prerendering and their sites are much slower as a result because it results in an extra round trip to the server before anything can be rendered. I found tons of examples of this in the wild. E.g. appwrite/console#722

Describe the proposed solution

Check if fetch is called. If it's not, then automatically prerender the page even if prerender = true is not set

For this to work, pages would need to be crawled and rendered even if prerender = true is not set, which I think is the case today though I'm not certain

We'll also need to add an isStatic check to the adapter and do it only when true so that we don't have to worry about losing headers.

Alternatives considered

  • Deprecate SPA mode. It's often used as a shortcut rather than addressing issues when they're hit. People just opt out of SSR as a quick and dirty fix, but users suffer for it. However, it can ease deployment to hosts that offer static deployment, so this may be a bit of a drastic measure
  • Tell users to turn on prerendering in cases where fetch is not called, SSR is disabled, and prerendering is not enabled. We could either throw an error or log a big nasty warning rather than just automatically prerendering the page

Importance

nice to have

Additional Information

No response

@Rich-Harris
Copy link
Member

Why is fetch use a consideration here?

There's basically four reasons why you might need to dynamically generate the response even when rendering an app shell:

  • Rewriting the HTML
  • Custom headers
  • Side-effects in handle
  • Prerendering is impossible because of things like this

We can detect the first two and de-opt. We can't detect the second two, which makes this a breaking change. I think the best we can do for now is add a config option that opts you in to prerendering the app shell, and warn when it isn't set (and adapters would need to be updated to be aware of it)

@benmccann
Copy link
Member Author

What I'm talking about is not the app shell, which wouldn't help much, but the entire page. I'm also suggesting we do it only when a static adapter is used. This is not limited to adapter-static, but could be any adapter which opts-in to specify via a new API that it does not generate server-side methods such as svelte-adapter-azure-swa. Since these are static adapters, handle would not be invoked in production with or without this change, making all of these points moot.

To summarize the issue in the fourth point, the user has an auth check in handle, which fails during prerendering, but since there's no prerender = true that's fine since the page is not saved. This is the most perilous of the four points. However, it's important to note that this user is using adapter-cloudflare. For users of adapter-static, such a handle check would have to run at prerender time or would be a no-op. It's unlikely that they would write a handle function for adapter-static that they didn't expect to run during prerendering.

If we introduce a new API in the adapter to trigger this behavior, we can cut a major release of the adapter. For other users of SvelteKit this behavior will not be triggered, so we could avoid a major version bump of the core framework. Even if the ideal solution we come up with is breaking, we can put this on the 3.0 milestone and address it then, so I wouldn't let the breakingness of a solution stop us from trying to figure out what to do here.

The reason to look for fetch usage is that users may not be able to prerender if the data is dynamically changing and they don't want to rebuild and redeploy the site each time the data changes.

@dummdidumm
Copy link
Member

We first need to define what exactly "prerendering" means in this context, because it sounds like we have different understandings of to what extent things are/should be prerendered.

Prerendering the app shell means that a HTML file is spit out which contains a few script tags but is otherwise a blank page. I believe this is what Rich is referring to. I don't think that there's much to gain here because the additional miliseconds that it takes to go through the server before the blank page is returned are tiny in comparison to the added roundtrip wait that you have until the JS is fetched from the browser and its results are rendered.

"Real" prerendering means somehow magically still spit out HTML that looks like the final page (or a large subset of it) despite ssr and prerendering being false. I believe this is what Ben envisions, but I don't see how this should be possible - it sounds like we should instead educate people on how to prerender specific entry pages even if the rest of their app is an SPA.

@benmccann
Copy link
Member Author

I don't see how this should be possible

That's what I'm proposing above. Why do you think it wouldn't be possible? Is there a part of what I've suggested that doesn't work? If a page doesn't fetch any data, I'm not sure why you couldn't automatically prerender it with adapter-static.

@dummdidumm
Copy link
Member

I think it's just very confusing. "Hey, you disabled SSR and have a fallback, but in fact we're going to do the opposite and prerender this particular route, but only in adapter-static because there we have enough constraints to somewhat safely do this".

@benmccann
Copy link
Member Author

I sent some docs updates here: #11637

We already discouraged SPA mode though, so it's unclear how much this will help. I would definitely be interested in whether we can present some type of warning or something in scenarios where we detect that a page could be prerendered.

We should also revisit the discussion in #10886 and ensure we're giving people as much guidance as possible on how to get their apps to work server-side.

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

No branches or pull requests

3 participants