feat: server and client init hook

[paoloricciuti]

This provides an init hook that will be awaited on the server and on the client upon app initialisation. A couple of notes:

1. I went with the same current convention: if the same hook is provided by a specific one and an universal one (basically if it's exported from hooks.ts and hooks.{server|client}.ts then hooks.ts will have priority.
2. I noticed that in dev the init is not actually run on the server until the first request is made (i think this is because the server is created in the dev server middleware in vite)...i think is better to document this unless we actually want to change the behaviour.

This is still missing

• test coverage (also any ideas of how i can test that the init function is indeed called on the server? for the client i can expect some logs on the page but i don't have ideas for the server)
• documentation
• write tutorial (although that needs to happen on svelte.dev)

but i wanted to get this out to at least get an initial feedback.

It could close #6183 (it's not exactly what's described in the issue however)

closes #927

---

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. Changesets that add features should be minor and those that fix bugs should be patch. Please prefix changeset messages with feat:, fix:, or chore:.

Edits

• Please ensure that 'Allow edits from maintainers' is checked. PRs without this option may be closed.

[changeset-bot]

🦋 Changeset detected

Latest commit: a9c85b0

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	Minor

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

[paoloricciuti]

I've removed the init hook from the universal hooks file...currently it still works on the server but i will adapt the code after #13104 is merged to prevent that

[Rich-Harris]

preview: https://svelte-dev-git-preview-kit-13103-svelte.vercel.app/

this is an automated message

[paoloricciuti]

@Rich-Harris the init_server function is called multiple times by some adapters and in dev mode...that's why we need to store the inited value

[Rich-Harris]

server.init is called multiple times, but new Server shouldn't be, surely? Take the point about dev mode though, oops! When I removed it it had no bearing on the tests. Will revert

[Rich-Harris]

Actually no, I stand by it — while the server is created anew for each request, the options object isn't, unless it changed. Tracking inited is equivalent to tracking !!options.hooks, we don't need both as far as I can tell

[paoloricciuti]

server.init is called multiple times, but new Server shouldn't be, surely? Take the point about dev mode though, oops! When I removed it it had no bearing on the tests. Will revert

new Server is actually called multiple time in dev mode too but yeah after build it should be decently fine...but it's a minor complexity so yeah i think we can revert 😄

[paoloricciuti]

Actually no, I stand by it — while the server is created anew for each request, the options object isn't, unless it changed. Tracking inited is equivalent to tracking !!options.hooks, we don't need both as far as I can tell

You can but it will incur in race conditions with multiple workers (since multiple requests can be made while the app is waiting for init) you can probably fix it by immediately setting options.hook before calling init tho but I remember I still had trouble for some reason I can look into why if you prefer

[paoloricciuti]

Actually no, I stand by it — while the server is created anew for each request, the options object isn't, unless it changed. Tracking inited is equivalent to tracking !!options.hooks, we don't need both as far as I can tell

Now i remember: what happens is that if you create multiple servers at the same time this.#options.hooks will be false for every Server. They all wait on get_hooks and proceed to set this.#options.hooks and then to execute module.init.

We could do something like this

if (module.init && !!this.#options.hooks) {
    await module.init();
}

but that would always be false since we set this.#options.hooks the line before so in reality we should do

if (module.init && !!this.#options.hooks) {
    await module.init();
}

if(module.init && !!this.#options.hooks){
    this.#options.hooks = {
        handle: module.handle || (({ event, resolve }) => resolve(event)),
        handleError: module.handleError || (({ error }) => console.error(error)),
        handleFetch: module.handleFetch || (({ request, fetch }) => fetch(request)),
        reroute: module.reroute || (() => {})
    };
    await module.init();
}else{
    this.#options.hooks = {
        handle: module.handle || (({ event, resolve }) => resolve(event)),
        handleError: module.handleError || (({ error }) => console.error(error)),
        handleFetch: module.handleFetch || (({ request, fetch }) => fetch(request)),
        reroute: module.reroute || (() => {})
    };
}

but it's a lot of duplication and i liked the boolean solution more

[Rich-Harris]

Ah, interesting. I'm surprised the race condition can manifest in real life since we're just importing local modules. Would just really love to be able to avoid hiding more state in virtual modules, because it makes it quite hard to understand how everything fits together. Just pushed something that seems to work, though I have to run now...

[paoloricciuti]

Ah, interesting. I'm surprised the race condition can manifest in real life since we're just importing local modules. Would just really love to be able to avoid hiding more state in virtual modules, because it makes it quite hard to understand how everything fits together. Just pushed something that seems to work, though I have to run now...

I mean we could just keep the inited boolean state locally instead of the virtual module (it should be more readable than the promise thing) but i trust you more than me if you think this is better i can fix the lint and be good with it 😄

[Rich-Harris]

I mean we could just keep the inited boolean state locally

I tried that, it doesn't work because the next await server.init(...) returns before the hooks have been populated. Having every init call await the same promise is the right design, I think, even if it took some trial and error to get there 😅

[paoloricciuti]

@Rich-Harris it would be good to also have #13104 merged...currently an init in hooks.ts would override hooks.server.tsand that pr fixes that (with some extra changes either to this PR or to that PR)

[eltigerchino]

would this close #927 as well?

[paoloricciuti]

would this close #927 as well?

I would say yes

[dummdidumm]

would this close #927 as well?

I don't think it will close it completely - some those use cases sound like they want to be able to do stuff at a lower level before the SvelteKit runtime even starts.

[paoloricciuti]

@Rich-Harris i don't know how this was not a conflict but we had to update the exports in get_hook for this PR to properly work after #13104...just pushed a fix but it was too late...opening a new pr