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

Fail prerendering if page attempts to access session in load - fixes #3722 #4811

Conversation

tcc-sejohnson
Copy link
Contributor

@tcc-sejohnson tcc-sejohnson commented May 4, 2022

It doesn't make sense to prerender a page that tries to access session, as it can never be populated by the server. Similarly to how we throw if we try to prerender a page that has a mutative shadow endpoint, we now throw if a page tries to access session during prerendering. This exception can be skipped by setting continueOnError to true, but it will still warn users by logging the error to the console. Fixes #3722.

As is, this will fail on server-side requests while in dev as well (so that developers don't have to wait until they try to build for prod to get slapped in the face with it).

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 pnpx changeset and following the prompts. All changesets should be patch until SvelteKit 1.0

@changeset-bot
Copy link

changeset-bot bot commented May 4, 2022

🦋 Changeset detected

Latest commit: ad3d599

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

@tcc-sejohnson
Copy link
Contributor Author

Hmm. Strange. I linted/checked locally...

I'll run again and push if necessary.

@Rich-Harris Rich-Harris merged commit 00a6c56 into sveltejs:master May 23, 2022
@Rich-Harris
Copy link
Member

excellent, thank you!

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.

getSession() does not get called on page load. should warn user
2 participants