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

Accept Generic Parameters for FormData, Response and Request #11

Closed
karlhorky opened this issue Feb 20, 2023 · 3 comments
Closed

Accept Generic Parameters for FormData, Response and Request #11

karlhorky opened this issue Feb 20, 2023 · 3 comments

Comments

@karlhorky
Copy link

karlhorky commented Feb 20, 2023

Hey @mattpocock 👋 Hope you're well!

Looks like a cool project! 👀

Wonder if you'd be open for enabling some DX improvements such as enabling FormData, Response and Request to take generic type parameters.

These have been requested in TypeScript itself, but it seems like the requests won't be implemented:

Motivating Examples

FormData

type HttpMethod = 'GET' | 'POST' | 'PUT' | 'DELETE';

type Options<Method> = {
  method?: Method;
}

export async function fetchApi<Path extends string, Method extends HttpMethod>(
  path: Path,
  options: Options<Method> & {
    formData: `${Method} ${Path}` extends `POST /exercise-checks`
      ? { id: string, file: File } // 👈 Could be FormData<{ id: string, file: File }>
      : `${Method} ${Path}` extends `PUT /exercise-checks/${number}`
      ? { file: File } // 👈 Could be FormData<{ file: File }>
      : never
  }): Promise<
    `${Method} ${Path}` extends `POST /exercise-checks`
      ? { id: string } | { errors: string[] }
      : `${Method} ${Path}` extends `PUT /exercise-checks/${number}`
      ? { id: string } | { errors: string[] }
      : never
  > {
  return '' as any;
}

const file = new File([''], '');
const goodFormData = { id: '1', file: file }; // 👈 Could be new FormData() + .append
const badFormData = { incorrectProperty: false }; // 👈 Could be new FormData() + .append

// ✅ No errors
await fetchApi('/exercise-checks', { method: 'POST', formData: goodFormData })
// ✅ No errors
await fetchApi('/exercise-checks/1', { method: 'PUT', formData: { file: file } })
// ✅ Errors, incorrect method
await fetchApi('/exercise-checks/1', { method: 'PUTzzzzz', formData: { file: file } })
// ✅ Error, incorrect path
await fetchApi('/xxxxx', { method: 'PUT', formData: { file: file } })
// ✅ Error, incorrect form data
await fetchApi('/exercise-checks/1', { method: 'POST', formData: badFormData })

TypeScript Playground

Response and Request

Before:

export async function POST(request: Request): Response {
  const body = await request.json();
     // ^? const body: any ❌
  return new Response(JSON.stringify({
    abc: 123, // ❌ POST function return type cannot check type of response body object
  }))
}

After:

type RequestBody = { zzz: number };
type ResponseBody = { abc: number };

export async function POST(request: Request<RequestBody>): Response<ResponseBody> {
  const body = await request.json();
     // ^? const body: { zzz: number } ✅
  return new Response(JSON.stringify({
    abc: 123, // ✅ type checked
  }))
}

Caveats

  1. Arguably, the Request body is unknown until runtime, so this is better handled instead with Zod or some other runtime validation. But for quick/simple projects without this validation, it's nice to be able to type the request body type.
@mattpocock
Copy link
Owner

mattpocock commented Feb 20, 2023

Hey! Thanks for your detailed issue, much appreciated.

I think I'd prefer not to add generics to Request, Response etc. The main reason is that I want to err towards correctness, not usability. When you call Request.json(), you truly don't know what you're going to get.

@mattpocock mattpocock closed this as not planned Won't fix, can't repro, duplicate, stale Feb 20, 2023
@karlhorky
Copy link
Author

karlhorky commented Feb 20, 2023

Ok, there are 3 points here:

  1. FormData - enabling checking of the shape of data inside an instance of FormData
  2. Request - arguably, validation should be done with Zod or similar, as you mention
  3. Response - this is for type checking responses returned from functions in a statically-analyzable way (eg. checking return values of API handler functions) - also mentioned this here

So I think your comment above only relates to point 2.

I think the highest value is probably the Response generic parameters. The Response type from @types/express has prevented a lot of bugs from happening in our applications.

But maybe this is not the focus of the @total-typescript/ts-reset library.

@darcyrush
Copy link

darcyrush commented Oct 24, 2024

@karlhorky I wonder if you (or even @mattpocock ) with your typing knowledge can help with a tangential question which is out of scope of the ts-reset library;

In @types/express both the ResBody and ReqBody types default as any

interface Request<
      P = core.ParamsDictionary,
      ResBody = any,
      ReqBody = any,
      ReqQuery = core.Query,
      Locals extends Record<string, any> = Record<string, any>
  > extends core.Request<P, ResBody, ReqBody, ReqQuery, Locals> {}

How would you use declaration merging to change these to unknown instead?

I have tried variations of the following, which can augment properties but not override;

declare global {
  namespace Express {
    interface Request extends Record<string, unknown> {
      addition: "a" | "b"; // Augments ok
      body: unknown; // Remains any
    }
  }
}

I understand the risks with overriding the definitions of an external library

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

No branches or pull requests

3 participants