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

rename prerendering to building, remove config.kit.prerender.enabled #7762

Merged
merged 3 commits into from
Nov 23, 2022

Conversation

Rich-Harris
Copy link
Member

@Rich-Harris Rich-Harris commented Nov 22, 2022

Follow-up to #7716. This PR makes it possible to add route-level configuration in future that can be used inside adapters to enable things like granular control over things like runtime and region.

It's a breaking change, because it means we need to remove config.kit.prerender.enabled to ensure that it's possible to run the app at build time.

It also renames prerendering to building.

How to migrate

Everywhere where you used prerendering, you now use building - switching the import is enough:

-import { prerendering } from '$app/environment';
+import { building } from '$app/environment';

// ...
-if (prerendering) {
+if(building) {
  // ..
}  

If you used config.kit.prerender.enabled, remove that config setting from svelte.config.js:

export default {
  kit: {
    prerender: {
-    enabled: false
    }
  }
}

You likely used config.kit.prerender.enabled so that no code is run during build. Now, if any code should not run during build (for example, connecting to a production database), it can be wrapped in an if block:

+import { building } from '$app/environment';

+if (!building) {
  do_some_setup_work();
+}

You only need to do that for top-level code (in other words code that is executed just by loading the file), things inside load for example are not run during build, unless you opted into prerendering (at which point you expect load to be run).

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 Nov 22, 2022

🦋 Changeset detected

Latest commit: fcb9f50

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

This PR includes changesets to release 1 package
Name Type
@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

@@ -77,17 +77,17 @@ declare module '$app/environment' {
export const browser: boolean;

/**
* Whether the dev server is running. This is not guaranteed to correspond to `NODE_ENV` or `MODE`.
* SvelteKit analyses your app during the `build` step by running it. During this process, `building` is `true`.
Copy link
Member

@dummdidumm dummdidumm Nov 22, 2022

Choose a reason for hiding this comment

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

Suggested change
* SvelteKit analyses your app during the `build` step by running it. During this process, `building` is `true`.
* SvelteKit analyses your app during the `build` step by running it. During this process, `building` is `true`.
* You can use this to wrap any code that should not be executed during build in a `if (!building) {...}` block.

Related: It might also be good to note in the page options docs that building will be true while prerendering. I also think it makes sense to explain somewhere how exactly the semantics of building are - for example, that this means top level code is run just because things are loaded, but that functions like load are not executed, unless at a later stage during prerendering.

Copy link
Member Author

Choose a reason for hiding this comment

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

agree, added some stuff to the docs

@dummdidumm
Copy link
Member

How many people have disabled that setting? I guess not many, because it works flawlessly even if you don't prerender anything, except when you do eager db setups, in which case you are probably already doing if (prerendering) {..}? Is there value in having a similar setting so that that you can opt out of build time analysis?

@Conduitry
Copy link
Member

I had disabled it in apps (that I was first porting from Sapper to SvelteKit a year ago), but I don't remember what specific problem I was avoiding by doing so. There aren't any eager database connections I had to avoid. But in any case, it looks like I don't need this configuration anymore, because the apps work fine without disabling it.

@Rich-Harris
Copy link
Member Author

I posted a link to the discussion in the two (well-populated) issue threads where this had come up in the past to get maximum visibility. Lots of 👍 reactions — tl;dr is I think we're in the clear. If we need to add some opt-out in future we can consider it but we shouldn't do it defensively

@Rich-Harris Rich-Harris merged commit a9a7e98 into master Nov 23, 2022
@Rich-Harris Rich-Harris deleted the gh-7716 branch November 23, 2022 14:34
@sdekna
Copy link

sdekna commented Nov 25, 2022

This change is a breaking change that broke a lot of apps, but no explanatory migration procedure is presented, is it possible to have one? I couldn't find my way through this migration.

@dummdidumm
Copy link
Member

I updated the PR description with a more descriptive migration guide.

@sdekna
Copy link

sdekna commented Nov 25, 2022

Many thanks man!
I think as the framework is approaching 1.0, it would be helpful to host a releases page somewhere on the website where every new release is announced and any breaking changes or notes explained... This might help.
Thanks again.

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.

4 participants