initial types testing

[ignatiusmb]

Now that we're more or less settling in on our types structure, a typings test would make sure that we have them reliable and don't break on some changes that might've slipped through. These are just basic tests for now but I've made sure to include some of the cases where it's prone to breakage. Outstanding issues will be dealt on later.

P.S. @dummdidumm we've talked about using tsd then, but I have yet to find a complex case where we'd need those fancy APIs with a huge dependency tree as well. We can always add and adjust the test later if we need to, and I think this should be enough for most of our cases right now.

Some questions TBD:

• undefined values causes error, might be an oversight but it's not exactly part of JSONValue either, but I'm pretty sure we'll have to change this behavior
• as nice as JSDoc and handwritten types goes, it has its limits, and we might be approaching them soon with the amount of conditionals we have for RequestHandlerOutput. A proposed solution and probably the easiest one would be to export an identity function so we can work more freely with our types. I haven't found a good strong problem and solution pair that needs this specific approach for it to work though, but one of the nice things that would be apparent with identity functions would be that (assuming VSCode as the base case here) users—both JS/TS—wouldn't need to manually type their config, load, or request handlers anymore. I'm not too strong on this for now, but it would be really nice to have.

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

• It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
• This message body should clearly illustrate what problems it solves.
• Ideally, include a test that fails without this PR but passes with it.

Tests

• Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

• If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpx changeset and following the prompts. All changesets should be patch until SvelteKit 1.0

[changeset-bot]

⚠️ No Changeset found

Latest commit: b2391a9

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[dummdidumm]

Oooh this is nice! Agree that we should keep it simple. Regarding your questions:

• undefined - I guess it should be part of the type. AFAIK undefined values are filtered out when serializing, so while they are not part of the output, they are still valid input
• identity function - what exactly do you mean by that? A function that the user should use to get the correct typing more easily? Could you give a code example?

[ignatiusmb]

For the identity function, yeah more or less what you explained there. I haven't found a nice solution yet but I did find one and it's mostly for #1997, Because we're asking users to type the functions themselves, we also need them to pass in the generics for type-checking to properly work like export const get: RequestHandler<any, MyPostInterface>, then the return body can receive an object of type MyPostInterface without erroring like right now. It doesn't work if it's only export const get: RequestHandler and I think we might be able to infer the return body using this identity function.

A code example for the above

// src/routes/endpoint.ts
import { request } from '@sveltejs/kit';

export const get = request(({ /* usual destructuring */ }) => {
  // usual code handling

  // usual return
  return {
    // ...
  };
});

It would abstract all the complicated type handling in that request identity function. It's optional as well so users can still just do export const get: RequestHandler = () => { ... } if they want to, something something progressive enhancement.

---

A code example with config

// somewhere in kit source code
export const config = (v: Config) => v;

// in user's svelte.config.js
import { config } from '@sveltejs/kit';

export default config({
  // ... autocomplete here
})

We could even make the config accept a generic for other apps to extend the kit config and add their own flavor to it.

[Rich-Harris]

This looks great. One downside of the identity function approach is we lose the ability to do this: #4120

[Rich-Harris]

I also think that an identity function obscures what's actually happening (you can't tell it's just an identity function by looking at it), and risks adding confusion ("why does this person's tutorial use this funny request function when this other person's doesn't?"), so I think I'm generally sceptical of that approach. We shouldn't let the TypeScript tail wag the JavaScript dog

[ignatiusmb]

We shouldn't let the TypeScript tail wag the JavaScript dog

Always fascinated to see the analogies that comes up around Rich, and this is probably the most amusing one so far. Yeah, those are some solid arguments, especially the different approaches that ends up taking away the consistency of a SvelteKit application between codebases and tutorials.

We'll keep them as is then, I think we can still strongly-type them in some ways.

[benmccann]

LGTM. typings sounds a bit funny to me as a directory name, but I can't think of anything better, so am fine with it