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

Non-SSR'd pages should be prerendered by default #3966

Closed
Rich-Harris opened this issue Feb 16, 2022 · 4 comments · Fixed by #8131
Closed

Non-SSR'd pages should be prerendered by default #3966

Rich-Harris opened this issue Feb 16, 2022 · 4 comments · Fixed by #8131
Labels
feature / enhancement New feature or request p2-nice-to-have SvelteKit cannot be used by a small number of people, quality of life improvements, etc.
Milestone

Comments

@Rich-Harris
Copy link
Member

Describe the problem

While we recommend that people use SSR as widely as possible, some apps don't. In these cases, there's no obvious reason not to prerender the shell page.

Describe the proposed solution

If a page is rendered with ssr: false, don't bail out during prerendering here, unless export const prerender is explicitly false (to provide an escape hatch when the app is doing something funky in handle):

if (!leaf.prerender && state.prerender && !state.prerender.all) {
// if the page has `export const prerender = true`, continue,
// otherwise bail out at this point
return new Response(undefined, {
status: 204
});
}

Alternatives considered

The alternative is to continue dynamically server-rendering empty pages. Totally fine, since rendering an empty page is cheap, but wasteful nonetheless.

Importance

nice to have

Additional Information

No response

@Rich-Harris Rich-Harris added this to the 1.0 milestone Mar 5, 2022
@benmccann benmccann added the feature / enhancement New feature or request label Mar 17, 2022
@benmccann benmccann added the p2-nice-to-have SvelteKit cannot be used by a small number of people, quality of life improvements, etc. label Jul 29, 2022
@Rich-Harris Rich-Harris modified the milestones: 1.0, whenever Jul 30, 2022
@Rich-Harris Rich-Harris modified the milestones: whenever, 1.0 Dec 10, 2022
@Rich-Harris
Copy link
Member Author

occurs to me that this is technically a breaking change, so added to 1.0

@Tal500
Copy link
Contributor

Tal500 commented Dec 12, 2022

Though this feature sounds awesome, this amplifies the problems with removing the option of disabling prerendering, as I said:

Today, you can disable that with config.kit.prerender.enabled = false. But if we're leaning more in this direction, we would probably need to get rid of that option.

For my testing, I like to build some projects locally with prerendering disabled (for debugging prerendering issues and also for faster build time). Therefore, removing the options to (temporary) disable prerendering is not preferable for me, and I believe also for other people.

I do agree that this configuration in svelte.config.js is problematic. As a compromise, maybe except some environment variable to disable prerendering or such?

Originally posted by @Tal500 in #7716 (comment)

Though this problem was amplified, the solution seems to be more clear now - perhaps add a configuration of prerendering, to be one of these three states:

  1. 'disabled': No prerendering, ignoring each page settings.
  2. 'strict': Prerender each page if and only if in the page settings prerender === true.
  3. 'lax': Prerender each page if and only if in the condition you mentioned in this issue is happening(can be changed in the future to be even more laxed).

Obvious note: This issue suggests to make the third option the default, while currently the second one is the default.

@Rich-Harris
Copy link
Member Author

I'm not totally sure I see the problem — you could prevent the prerendering from happening just by passing entries: [] (which you're probably already doing if you can't run handle inside build) or adding prerender = false alongside ssr = false.

@Tal500
Copy link
Contributor

Tal500 commented Dec 12, 2022

I'm not totally sure I see the problem — you could prevent the prerendering from happening just by passing entries: [] (which you're probably already doing if you can't run handle inside build) or adding prerender = false alongside ssr = false.

OK, missed that option. Tnx

Rich-Harris pushed a commit that referenced this issue Dec 13, 2022
…8131)

* [breaking] prerender shells when ssr false and prerender not false

Closes #3966

* fix

* keep implicitly rendered route in the manifest

* there's no 1

* additional check

* omit type check for now, uncovered a unreleated issue
dummdidumm added a commit that referenced this issue Dec 14, 2022
Rich-Harris added a commit that referenced this issue Dec 14, 2022
* [docs] note ssr/prerender dependency

Docs for #3966

* move after "when not to prerender", shrink word count

Co-authored-by: Rich Harris <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature / enhancement New feature or request p2-nice-to-have SvelteKit cannot be used by a small number of people, quality of life improvements, etc.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants