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

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

Closed
cdcarson opened this issue Sep 21, 2022 · 3 comments · Fixed by #7002
Labels
bug Something isn't working types / typescript

Comments

@cdcarson
Copy link
Contributor

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:

export interface 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>;
}

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
Copy link
Contributor Author

cdcarson commented Sep 22, 2022

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:

if (fs.existsSync(types_dir)) {
for (const file of walk(types_dir)) {
const dir = path.dirname(file);
if (!expected_directories.has(dir)) {
rimraf(file);
}
}
}

@cdcarson
Copy link
Contributor Author

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?

@dummdidumm dummdidumm added types / typescript bug Something isn't working labels Sep 23, 2022
@dummdidumm
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working types / typescript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants