add a SvelteKit namespace for app-level types (#3569)

[Rich-Harris]

Ref #3569.

This is a breaking change, as it affects how Load and ErrorLoad are typed if you're using generic arguments. If you're not, TypeScript will likely start yelling at you because session and locals etc are no longer typed as any.

By creating an app-level namespace, we're able to provide types for event.locals, event.platform, page.session and page.stuff throughout the app with no duplication. One caveat — it's no longer possible to specify input stuff and output stuff separately for a load function (unless you create your own type, I suppose) which might be a nuisance if you're ultra-pedantic. Personally I think the ergonomics are vastly better this way.

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]

🦋 Changeset detected

Latest commit: 52b31ee

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

This PR includes changesets to release 2 packages

Name	Type
create-svelte	Patch
@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

[netlify]

✔️ Deploy Preview for kit-demo canceled.

🔨 Explore the source changes: 52b31ee

🔍 Inspect the deploy log: https://app.netlify.com/sites/kit-demo/deploys/61faa2b3349ddd0008ae71e1

[Conduitry]

7:12 error An empty interface is equivalent to {} @typescript-eslint/no-empty-interface

How helpful.

[dummdidumm]

I really like this change, especially because it allows us to remove so many type-level code and the generics hack.

Couple of thoughts:

• is app.d.ts the best name? You can add anything that is global/ambient there, I slightly prefer the previous name, but I'm not passionate about it and can live with it either way
• Is App a good namespace name? My fear is that it's rather generic and the possibility of namespace clashes might be higher than with another name. What about $App since that somewhat corresponds to all the $app/.. imports?
• Are all of these globally valid? For example Stuff does differ depending on your route. You might have foo inside stuff in one part of your app and bar in another part. This is not possible to express with this way - but I don't know how we could express this either. The only way I can think of is people wrapping the stuff store and reexporting it with a more specific type like export const myStuff = stuff as Writable<SomeSubStuff>

[dummdidumm]

Any possibility that someone would want this to be a primitive type which can't be expressed as an object-like interface?

[Rich-Harris]

No, they're all required to be objects

[Rich-Harris]

is app.d.ts the best name?

Ultimately it doesn't really matter — you could rename it, or create a global.d.ts alongside it, and everything would continue to work. I kinda liked the implication that these interfaces relate to your app specifically (rather than, like, the environment) but I don't have strong feelings about it

Is App a good namespace name? My fear is that it's rather generic and the possibility of namespace clashes might be higher than with another name.

Would those clashes ever be coming from dependencies? I haven't ever seen that happen but maybe there are packages that do that? If it's only likely to conflict with the user's own namespaces then I think that's probably fine, they would just need to rename stuff (and even then, only if their own namespace contained interfaces with conflicting names).

$App to me implies a closer connection to the $app modules than really exists. I initially used SvelteKit per #3569 (comment), but it felt weird that those interfaces weren't coming from SvelteKit.

Are all of these globally valid? For example Stuff does differ depending on your route.

It can differ, but since it's globally accessible as $page.stuff I think we want to encourage people to treat it as such. I'd expect the cases where it differs from route to route to be fairly rare and somewhat contrived.

[dummdidumm]

Would those clashes ever be coming from dependencies? I haven't ever seen that happen but maybe there are packages that do that? If it's only likely to conflict with the user's own namespaces then I think that's probably fine, they would just need to rename stuff (and even then, only if their own namespace contained interfaces with conflicting names).

As soon as something make TS load a namespace it's part of the global types, and they can then clash. Doesn't matter where they come from. They also do need to have the same interface definitions inside for the clash to be noticeable though, which I guess would be very rare. I don't know, maybe this fear is unfounded.

$App to me implies a closer connection to the $app modules than really exists. I initially used SvelteKit per #3569 (comment), but it felt weird that those interfaces weren't coming from SvelteKit.

👍

It can differ, but since it's globally accessible as $page.stuff I think we want to encourage people to treat it as such. I'd expect the cases where it differs from route to route to be fairly rare and somewhat contrived.

Agree that the benefits outweigh the edge cases of pedantic typing wizards

[ignatiusmb]

This is refreshing to see. adapter-cloudflare docs needs updating as well, but other than that, this looks good!