sveltejs / kit

web development, streamlined
https://svelte.dev/docs/kit
MIT License
18.68k stars 1.93k forks source link

Make App.d.ts route id aware #11925

Open Kworz opened 8 months ago

Kworz commented 8 months ago

Describe the problem

Defining globals for PageData is fine for most of the time. But when you are splitting your app in 2 or more named layouts with with specific data being loaded in +layout.ts / +layout.server.ts it's frustrating to have to force non-null checking for data that will always be there.

For example:

declare global {
    namespace App {
        /** Local data populated by Handle hook */
        interface Locals
        {
            prisma: PrismaClient;
            s3: S3Client;

            user: User | null;
            session: Session | null;

            appSettings?: AppSettings; // this is never populated in /install route
            userSettings?: UserSettings; // this is never populated in /install route
        }

        /** Page data populated by layouts load function */
        interface PageData
        {
            user: User | null;
            session: Session | null;

            appSettings?: AppSettings // this is never populated in /install route
            userSettings?: UserSettings // this is never populated in /install route
        }
    }
}

Leads to having ESLint being completely lost here, as data is there but typed as optional.

Capture d’écran 2024-03-01 à 09 36 54

Describe the proposed solution

The actual way of defining global app types should not be modified as it works fine for almost everyone. We could add something like:

declare global {

    namespace App {

        // interface Error {}
        // interface Platform {}
                 // interface PageData {} This becomes useless as it is filled with context types

        /** Local data populated by Handle hook */
        interface Locals
        {
            prisma: PrismaClient;
            s3: S3Client;
        }

        /** 
         * Define fine grained types based on route context. 
         * Routes inherits global types and their corresponding context types 
         */
        interface Context {

            /** 
                           * Define data based on route context 
                           * `/app/*` would match all routes that begins with /app
                           */
            [`/app/*`] :  {

                locals: {
                    appSettings: AppSettings;
                    userSettings: UserSettings;
                    user: User;
                    session: Session;
                }

                pageData: {
                    user: User;
                    session: Session;
                    appSettings: AppSettings;
                    userSettings: UserSettings;
                }
            }
        }
    }
}

Alternatives considered

As stated in the docs Use optional properties for data that is only present on specific pages. works fine tbh, juste let ESLint scream at you and be confident about your data structure.

Importance

would make my life easier

Additional Information

No response

jasonlyu123 commented 8 months ago

I don't quite understand the use case for this. If you're in the routes files you can use the data props instead of the page store. If you're in a non-route component or a parent layout file, there isn't a specific path to "fine-grain" the global type. Can you provide an example repo that showcases the problem you're facing?

Kworz commented 7 months ago

I'm facing this issue in this repo https://github.com/Kworz/mink in the hooks.server.ts we can see that not all the locals are filled with data due to their route context, that is why types based on routes would be a great addition

https://github.com/Kworz/mink/blob/dac63a6a3d44f492d02dea5c3af8404c28d5eda9/src/hooks.server.ts#L10-L83