kit icon indicating copy to clipboard operation
kit copied to clipboard

Generated `PageData` type is `never` when a `page.server.ts` `load` returns `void`, regardless of data returned from layouts further up the tree

Open cdcarson opened this issue 3 years ago • 2 comments

Describe the bug

A minor typing thing, but it may be a bug in how types are generated .

Given the routes...

src/routes
├── +layout.server.ts
├── +page.svelte
├── bad
│   ├── +page.server.ts
│   └── +page.svelte
├── gate-authenticated.ts
└── good
    ├── +page.server.ts
    └── +page.svelte

...and the loads...

// src/routes/+layout.server.ts
export const load: LayoutServerLoad = () => {
  return {foo: 'bar'}
}

// src/routes/bad/+page.server.ts
export const load: PageServerLoad = (event): void => {
  // just to show why one might return void here 
  gateAuthenticated(event);
}

// src/routes/good/+page.server.ts
export const load: PageServerLoad = (event) => {
  gateAuthenticated(event);
  return {};
}

...the generated PageData types for src/routes and src/routes/bad are:

(alias) type PageData = never

Only src/routes/good has the "correct" type (since, I assume, the load returns {} rather than void)...

(alias) type PageData = {
    foo: string;
}

Kit.ServerLoad is advertised as able to return void:

https://github.com/sveltejs/kit/blob/2776f338c37a57f98fee3d82ed7f56dc30bbedd6/packages/kit/types/index.d.ts#L373-L379

So, maybe the void should be removed? I'd be fine returning {} if necessary.

Weirdly, though, the void returned from src/routes/bad/+page.server.ts is apparently bricking the type for src/routes, up the tree.

Reproduction

https://github.com/cdcarson/sk-void-load-gen-types-issue.git

Logs

No response

System Info

System:
    OS: macOS 12.5.1
    CPU: (8) arm64 Apple M1
    Memory: 137.09 MB / 8.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 16.17.0 - ~/.nvm/versions/node/v16.17.0/bin/node
    npm: 8.15.0 - ~/.nvm/versions/node/v16.17.0/bin/npm
  Browsers:
    Chrome: 105.0.5195.125
    Safari: 15.6.1
  npmPackages:
    @sveltejs/adapter-auto: next => 1.0.0-next.78 
    @sveltejs/kit: next => 1.0.0-next.492 
    svelte: ^3.44.0 => 3.50.1 
    vite: ^3.1.0 => 3.1.3

Severity

annoyance

Additional Information

No response

cdcarson avatar Sep 21 '22 21:09 cdcarson

Weirdly, though, the void returned from src/routes/bad/+page.server.ts is apparently bricking the type for src/routes, up the tree.

This was due to .svelte-kit/types/src/routes/proxy+page.server.ts not being deleted when I moved src/routes/+page.server.ts to the bad folder. So this is a separate bug having to do with clearing out moved/deleted proxies. It's not related to the way Kit is inferring types.

This seems to be the source of the "deleted file" problem. It doesn't check whether the file (say +page.server.ts) exists, just the directory:

https://github.com/sveltejs/kit/blob/d19964a21f37255b4fa87d90acd604236199b4f4/packages/kit/src/core/sync/write_types/index.js#L40-L47

cdcarson avatar Sep 22 '22 13:09 cdcarson

As for the first issue (a void return from +page.server.ts resulting in never) I have gotten so far as giving myself a headache. One thing -- in other though similar contexts I've always had way much better luck defining functions as type rather than interface.

export type ServerLoad< 
 	Params extends Partial<Record<string, string>> = Partial<Record<string, string>>, 
 	ParentData extends Record<string, any> = Record<string, any>, 
 	OutputData extends Record<string, any> | void = Record<string, any> | void 
 > = (event: ServerLoadEvent<Params, ParentData>) => MaybePromise<OutputData>; 

Is there a reason we are defining the functions as interface?

cdcarson avatar Sep 22 '22 14:09 cdcarson

EnsureParentData (which we probably should rename during this) should also enclose the PageServerData type

dummdidumm avatar Sep 23 '22 09:09 dummdidumm