kit icon indicating copy to clipboard operation
kit copied to clipboard

Improve the typing of `$page`

Open DavidArchibald opened this issue 1 year ago • 3 comments

Describe the problem

I found it surprising that the default typing for $page is App.PageData & Record<string, any> and that refining it is so out of the way. I only knew how to refine the type of App.PageData because of a lot of experience already with declaration merging etc. However I was disappointed to see that Record<string, any> can't be disabled or refined. At the least I would want to be able to do App.PageData & Record<string, unknown> or even just App.PageData to be more type safe.

Furthermore I was surprised that App.PageData wasn't deduced from the return type of my +layout.ts. svelte-kit sync already creates a .svelte-kit/types/src/routes/$types.d.ts file that has a perfectly good PageData type that seems to accurately describe the resolved type of the root load function. However it doesn't use declaration merging to make sure that App.PageData and by extension $page uses it.


I understand children load functions can essentially merge into the page data so $page can be assumed to have extra properties but I don't want simple typos messing me up and Record<string, unknown> makes it easier to avoid it but App.PageData is even safer. For an example look at this:

// This is a typo of `realProperty` which in this example would be part of the `load` function.
// With `$page` typed as `App.PageData` this would be an error immediately saying that that property doesn't exist. I like this and you can always refine the type of `$page` through TypeScript's flow control analysis.
const someData = $page.data.realPorperty;

// With `$page` typed as `App.PageData & Record<string, unknown>` this is an error because `someData` is of type `unknown` but it expects `string.
// Currently this won't error at all though because the type of `$page` is currently `App.PageData & Record<string, any>` and so `someData`'s type is `any`.
processData(someData);

function processData(data: string) { ... }

I understand that export let data is an option for +page.svelte files but plenty of my components use import { page } from '$app/stores'; and I had thought that was idiomatic.

Describe the proposed solution

I pretty much am writing the code that would need to be added already. I'd be willing to PR this but wanted to make an issue first.

I think .svelte-kit/types/src/routes/$types.d.ts should add this to the end:

// Alias to make the declaration merge work.
// `PageData` is already defined earlier in this file.
type _PageData = PageData;

declare module "@sveltejs/kit" {
    namespace App {
        export interface PageData extends _PageData {}
    }
}

This is the top level route file and so it'll only augment for the top level load.

I almost feel like I'm missing something about why this wouldn't work so please let me know if this would be a problem. Maybe there's some way to turn off parent loads so this isn't always sound? However I've already added this to my code and had no issues so far.

Since it wasn't always like this, the only concern I can see is that isn't 100% backwards compatible. Say your load function returns { realProperty: { realData: number } }, if you have faulty code today like $page.realProperty.nonexistantProperty it'll work today and be typed as any. If the type is improved, this would be come an error. However this should only ever cause an issue with currently bugged code. If there's a deep merge situation going on with load functions or something the type could be improved in that case too. Ideally could be tested on the ecosystem to see how much of problem this is. Or it could be introduced only as part of a major version.

Then for my second feature proposal I'd suggest this change: In @sveltekit/kit/src.types/ambient.d.ts

declare namespace App {
    [...]

    // This type describes the options the configure the `PageData` type.
    export type PageDataOptions = {
        // Whether the page data should be considered exhaustive or not.
        // By default this is `false` and extra properties are allowed on the page.
        exhaustive: boolean;

        // Whether extra properties should be `unknown`.
        // By default this is `false` and extra properties are typed as `any`.
        useUnknown: boolean;
    }

    export interface PageDataConfiguration extends PageDataOptions {}
}

Then in the core @sveltekit/kit/src/exports/public.d.ts file, I would make this tweak:

export interface Page<...> {
    [...]

    data: App.PageData & (
        // The condition is written this way because we should never distribute and it has to be exactly `true`. The type `boolean` shouldn't count as exhaustive.
        true extends App.PageDataConfiguration['exhaustive'] ? {} : (
            true extends App.PageDataConfiguration['useUnknown'] ? Record<string, unknown> : Record<string, any>
        )
    );
}

Users would configure like so:

declare module "@sveltejs/kit" {
    namespace App {
        export interface PageDataConfiguration extends App.PageDataOptions {
            exhaustive: true;

            // This is redundant with exhaustive but it's probably not worth an error or anything.
            useUnknown: true;
        }
    }
}

This portion should be 100% backwards compatible because when unset the type is still App.PageData & Record<string, any>.

Alternatives considered

No response

Importance

nice to have

Additional Information

No response

DavidArchibald avatar Feb 01 '24 23:02 DavidArchibald

I ran in the same problem. I am currently working around that using the deprecated export let data; which seems to be getting perfect typing in vscode without the need of any type annotation. Not sure if it works because of how that feature was implemented or it is some magic performed by the vscode plugin.

tacone avatar Feb 16 '24 19:02 tacone

@tacone where is stated that export let data is deprecated?

Anyway, export let data results in type { [x: string]: any } for me, anyways...

The must frustrating thing is that this worked perfectly before.

Alia5 avatar Feb 16 '24 21:02 Alia5

@Alia5 can't find a reference, perhaps It just me was hallucinating. The $props method is documented as a replacement though, so it wouldn't be that surprising I guess.

In my app, export let data is typed as PageData. I put there no typing whatsoever. Note that I am using javascript+typescript checking, not plain typescript.

import { rpc } from '$lib/rpc';

export async function load() {
	return {
		users:  rpc.userList.query()
	};
}
<script>
	export let data;  // correctly typed as PageData
</script>

My package json:

  svelte: ^5.0.0-next.1
  svelte-check: ^3.6.0
  svelte-preprocess: ^5.1.3
  '@sveltejs/adapter-auto': ^3.0.0
  '@sveltejs/adapter-static': ^3.0.1
  '@sveltejs/kit': ^2.0.0
  '@sveltejs/vite-plugin-svelte': ^3.0.0

tacone avatar Feb 17 '24 08:02 tacone