Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LayoutServerLoad return type is not compatible with simple interface object #5963

Closed
zlepper opened this issue Aug 17, 2022 · 5 comments · Fixed by #5974
Closed

LayoutServerLoad return type is not compatible with simple interface object #5963

zlepper opened this issue Aug 17, 2022 · 5 comments · Fixed by #5974

Comments

@zlepper
Copy link

zlepper commented Aug 17, 2022

Describe the bug

If you explicitly declare the return type of the load function in +layout.server.ts, then you will be told by typescript that it doesn't match the expected return type. For example:

import type { LayoutServerLoad } from './$types';

export interface Session {
	authToken: string;
}

export const load: LayoutServerLoad = ({ locals }): { session: Session } => {
	return {
		session: {
			authToken: locals.authToken,
		},
	};
};

Specifically you get this nice error:

Error: Type '({ locals }: ServerLoadEvent<LayoutParams, null>) => { session: Session; }' is not assignable to type 'LayoutServerLoad'.
  Type '{ session: Session; }' is not assignable to type 'MaybePromise<void | JSONObject>'.
    Type '{ session: Session; }' is not assignable to type 'JSONObject'.
      Property 'session' is incompatible with index signature.
        Type 'Session' is not assignable to type 'JSONValue'.
          Type 'Session' is not assignable to type 'JSONObject'.
            Index signature for type 'string' is missing in type 'Session'.

Running npm run check will output the above error if you don't get it directly in your development environment

Reproduction

https://stackblitz.com/edit/sveltejs-kit-template-default-o1ssqr?file=src%2Froutes%2F%2Blayout.server.ts,package.json&terminal=dev

Stackblitz can't find the ./$types so it doesn't actually reproduce the error...

Logs

No response

System Info

System:
    OS: Windows 10 10.0.19044
    CPU: (64) x64 AMD Ryzen Threadripper PRO 3975WX 32-Cores
    Memory: 31.36 GB / 127.85 GB
  Binaries:
    Node: 16.13.1 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.15 - C:\Program Files\nodejs\yarn.CMD
    npm: 8.1.2 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Chrome: 104.0.5112.81
    Edge: Spartan (44.19041.1266.0), Chromium (104.0.1293.54)
    Internet Explorer: 11.0.19041.1566
  npmPackages:
    @sveltejs/adapter-auto: next => 1.0.0-next.57
    @sveltejs/adapter-cloudflare: next => 1.0.0-next.31
    @sveltejs/kit: 1.0.0-next.415 => 1.0.0-next.415
    svelte: ^3.44.0 => 3.49.0
    vite: ^3.0.0 => 3.0.8

Severity

annoyance

Additional Information

No response

@elliott-with-the-longest-name-on-github
Copy link
Contributor

https://github.com/tcc-sejohnson/kit-broken-layout-server-typings

Here's another repro. Turns out this can bite you even if you just have an explicitly typed variable as part of your return signature.

@Rich-Harris
Copy link
Member

Does changing it to a type help? I seem to recall JSONObject working with types but not interfaces:

-export interface Session {
+export type Session = {
	authToken: string;
}

Not that that's a solution — ideally it'd work either way — but it'd be useful information, and is a possible workaround

@dummdidumm
Copy link
Member

It works that way, yes, but it sucks to do that. This is essentially the reincarnation of that problem we had with the old types, which is why we introduced the BodyValidator type, only this time we probably can't use that.

@abdo643-HULK
Copy link

For deep interfaces type aliasing on the root like this type GetBody = { product: Product; jsonLd: Schema; } & { [key: string]: JSONValue }; works for now

@dummdidumm
Copy link
Member

dummdidumm commented Aug 17, 2022

Ok so here's my summary of the issue(s):
Right now the OutputData generic (which represent's the load return value) is typed as extending JSONObject. JSONObject contains an index signature. This fucks up TS when you pass in an interface (in this example: Index signature for type 'string' is missing in type 'Section'). I also found no way to relax the type signature in a way that the generic ServerLoad type accepts more types but errors on a invalid types. This means we can't type "this needs to be a JSON object" (if you do const foo: JSONObject = {session: null as Session}, that will fail with the same index signature error). I therefore think we should get rid of that JSONObject entirely and instead rely on runtime checks like in #5635 - this would also be more future proof if we in the future decide to allow more sophisticated object<->string converters.

Rich-Harris pushed a commit that referenced this issue Aug 17, 2022
* Fix params and parent data types

* add output generic to load function types, helps with #5963

* remove JSONObject, add event types to $types

* fix types

* changeset
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants