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

Prerendering overhaul #6392

Merged
merged 19 commits into from
Aug 30, 2022
Merged

Prerendering overhaul #6392

merged 19 commits into from
Aug 30, 2022

Conversation

Rich-Harris
Copy link
Member

@Rich-Harris Rich-Harris commented Aug 29, 2022

Migration

General

+server pages can be prerendered, too now, by adding the export const prerender = .. option to them. By default they are not prerendered, but any +server page that is referenced from a prerendered page inherits the prerender option of that page unless explicitly set otherwise. This might lead to a situation where you now get a "Cannot prerender a +server file with POST, PATCH, PUT, or DELETE" error. To fix it, add export const prerender = false; to the +server file where it happens, or move the mutative methods in a different file.

adapter-static

Instead of only checking the default for prerendering, the actual found routes are now checked for prerendering. If one of them is not prerenderable, an error is thrown unless you set the fallback option. If you get this error now, adjust your code so that

  • either the non-prerenderable routes are not crawled anymore
  • or the non-prerenderable routes are made prerenderable
  • or set the fallback option

PR description

#6356.

  • differentiate between prerenderable and non-prerenderable endpoints
  • exclude routes with prerender = true from generated manifests
  • allow 'auto' (prerender, but also include in manifests) as a value, alongside true (only prerender, exclude from manifests) and false (never prerender, include in manifests)
  • throw in adapter-static if any routes are prerender = false or prerender = 'auto' (or unknown, in the case of endpoints that are never fetched during prerendering)
  • docs
  • idk, probably some tests and stuff

Not in scope for this PR: removing config.kit.prerender.default and using defaults from +layout[.server].js files. That can be added to #6197 once this is merged.

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

  • Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

  • If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. All changesets should be patch until SvelteKit 1.0

@changeset-bot
Copy link

changeset-bot bot commented Aug 29, 2022

🦋 Changeset detected

Latest commit: 64a4c75

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@sveltejs/adapter-static Patch
@sveltejs/kit Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR


Prerendering happens automatically for any page with the `prerender` annotation:
Prerendering happens automatically for any `+page` or `+server` file with the `prerender` annotation:
Copy link
Member

Choose a reason for hiding this comment

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

should we add JS here to clarify that you can't add it to +page.svelte?

Suggested change
Prerendering happens automatically for any `+page` or `+server` file with the `prerender` annotation:
Prerendering happens automatically for any `+page` or `+server` JS file with the `prerender` annotation:

Copy link
Member Author

Choose a reason for hiding this comment

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

I was hoping the code sample would make it clear — adding JS muddies the waters a bit in the case where people are using TypeScript

Copy link
Member

Choose a reason for hiding this comment

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

There are some other places where it's +page/layout with the .js ending. We should probably make that consistent, either adding the .js file always or never (except for when we introduce it).


```js
/// file: +page.js/+page.server.js/+server.js
export const prerender = 'auto';
Copy link
Member

Choose a reason for hiding this comment

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

now that I've seen the docs and understand what this option does, I'll throw another name into the ring. how about 'partial'?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think I prefer autopartial doesn't have the right connotation to me, it sounds like the page is being partially prerendered

Copy link
Member

@dummdidumm dummdidumm left a comment

Choose a reason for hiding this comment

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

Looks good! Will adjust the minor suggestions I made myself and merge to get the related PR in good state


```js
/// file: +page.js/+page.server.js
/// file: +page.js/+page.server.js/+server.js
Copy link
Member

Choose a reason for hiding this comment

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

I was confused for a bit because I think "what kind of folder names are these?". Maybe instead +page.js, or +page.server.js, or +server.js?

Copy link
Member

Choose a reason for hiding this comment

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

Skipping this for now, seing that it's like this for all others as well.

packages/kit/types/private.d.ts Show resolved Hide resolved
packages/kit/src/runtime/server/page/fetch.js Outdated Show resolved Hide resolved
export const prerender = false;
```

Routes with `prerender = true` will be excluded from manifests used for dynamic SSR, making your server (or serverless/edge functions) smaller. In some cases you might want to prerender a route but also include it in the manifest (for example, you want to prerender your most recent/popular content but server-render the long tail) — for these cases, there's a third option, 'auto':
Copy link
Member

Choose a reason for hiding this comment

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

It took me a little while to understand why this works - why can I prerender AND SSR a route - until I thought "oh yes with [slug]". I don't know how to express this better, but maybe we can come up with something later.

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

Successfully merging this pull request may close these issues.

3 participants