Skip to content

Comments

[fix] prerendering always false in global code of hooks.js #4265#4322

Merged
Rich-Harris merged 13 commits intosveltejs:masterfrom
Theo-Steiner:4265/prerendering
Mar 14, 2022
Merged

[fix] prerendering always false in global code of hooks.js #4265#4322
Rich-Harris merged 13 commits intosveltejs:masterfrom
Theo-Steiner:4265/prerendering

Conversation

@Theo-Steiner
Copy link
Contributor

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.

This fixes the behavior documented in #4265, where prerendering imported from '$app/env' is false outside the actual hooks in hooks.ts even when called during prerender.
This occurs because the hooks module is imported by the same module that exports the override function that allows to set prerendering in the first place. Therefore, when the global code of the hooks module is executed as it is imported, prerendering can never be any other value than it's default value.

This PR addresses this behavior by extracting the setting of prerendering into its own module (notify_prerendering.js), which can be imported without triggering the global code of the hooks module. As the notify_prerendering.js module will never be used anywhere other than during prerendering, it is not included into the writeServer() output of the builder.

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

Alternatives considered

Instead of making these changes, this behavior could also be documented under $app/prerendering as people can circumvent the problem with a setup function as described in #4265.

@changeset-bot
Copy link

changeset-bot bot commented Mar 14, 2022

🦋 Changeset detected

Latest commit: 8f9ff86

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

@Theo-Steiner Theo-Steiner changed the title [fix] prerendering always false in global code of hooks.js [fix] prerendering always false in global code of hooks.js #4265 Mar 14, 2022
@Rich-Harris
Copy link
Member

Thank you. There's actually a simpler fix, which is more in line with plans for future changes (namely the addition of scoped __middleware.js files, which would likely take the place of hooks.js) — we import the hooks lazily rather than eagerly. I've updated the branch

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.

2 participants