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 type parameters for NextResponse #45943

Closed
karlhorky opened this issue Feb 15, 2023 · 6 comments · Fixed by #47526
Closed

Accept generic type parameters for NextResponse #45943

karlhorky opened this issue Feb 15, 2023 · 6 comments · Fixed by #47526
Labels
TypeScript Related to types with Next.js.

Comments

@karlhorky
Copy link
Contributor

karlhorky commented Feb 15, 2023

Describe the feature you'd like to request

Hi, first of all thanks for your amazing effort on Next.js, great framework 🙌

With the new App Routes coming courtesy of @wyattjoh, @ijjk and @timneutkens with the merge + publish to canary of #45716, the following pattern is now possible in an App Route for eg. an API handler:

import { NextRequest, NextResponse } from 'next/server';

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

export async function POST(request: NextRequest) {
  const body: RequestBody = await request.json();
  return NextResponse.json({
    abc: 123, // ❌ POST function return type cannot check type of response body object
  });
}

These are using the NextRequest and NextResponse types created by @javivelasco and others.

But the ResponseBody type are not able to be checked easily here.

Ideally, NextResponse would accept generic type parameters (just like the NextApiResponse type currently does for API routes in the pages/api/ directory).

Describe the solution you'd like

It would be amazing to be able to pass generic type parameters into the NextResponse type, enabling a pattern like this:

import { NextRequest, NextResponse } from 'next/server';

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

export async function POST(request: NextRequest<RequestBody>): NextResponse<ResponseBody> {
  const body: RequestBody = await request.json();
  return NextResponse.json({
    abc: 123, // ✅ POST function return type checks type of response body object
  });
}

Prior Art

Caveats

Edit: Removed this section, because this issue was originally about allowing generic type parameters with NextRequest as well, but this has since changed to only encompass NextResponse

  1. Arguably, the NextRequest 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.

Describe alternatives you've considered

An alternative would be for TypeScript to make the Response and Request interfaces accept generic type parameters:

However, this may be unlikely to happen - a similar suggestion for FormData has been not implemented since 2021:

@github-actions github-actions bot added the TypeScript Related to types with Next.js. label Feb 15, 2023
@karlhorky karlhorky changed the title Accept generic type parameters in NextResponse and NextRequest Accept generic type parameters for NextResponse and NextRequest Feb 15, 2023
@timneutkens
Copy link
Member

Checking the body should live outside of the type Next.js gives you because typing the incoming body gives a false sense of type safety. E.g. I can fetch('yourapi', { body: JSON.stringify({a: 'a', b: 'b'}) }) and your API would not handle it correctly because the body was not validated / sanitized. The recommendation for this would be to use libraries like zod that allow you to define a shape that is validated and then use the inferred type.
I think body needs to be unknown instead of any though.

For the response body you have a point, however I'm not sure if it should be exactly that way, because NextResponse is an extension of Response and Response doesn't take a generic, which when that changes would become a problem.

I had a quick look into it and the change would be something like this:

export declare class NextResponse<B = void> extends Response {
    [INTERNALS]: {
        cookies: ResponseCookies;
        url?: NextURL;
        // Parameter needs to be used in the class in order for it to run type checking
        B: B
    };
    constructor(body?: BodyInit | null, init?: ResponseInit);
    get cookies(): ResponseCookies;
    // json takes in generic.
    static json<T>(body: T, init?: ResponseInit): NextResponse<T>;
    static redirect(url: string | NextURL | URL, init?: number | ResponseInit): NextResponse;
    static rewrite(destination: string | NextURL | URL, init?: MiddlewareResponseInit): NextResponse;
    static next(init?: MiddlewareResponseInit): NextResponse;
}

Will discuss with the rest of the team.

@karlhorky
Copy link
Contributor Author

karlhorky commented Mar 5, 2023

Checking the body should live outside of the type Next.js gives you because typing the incoming body gives a false sense of type safety. E.g. I can fetch('yourapi', { body: JSON.stringify({a: 'a', b: 'b'}) }) and your API would not handle it correctly because the body was not validated / sanitized. The recommendation for this would be to use libraries like zod that allow you to define a shape that is validated and then use the inferred type.

Makes sense. Real runtime validation (eg. using Zod) is indeed better. We're teaching this now in the bootcamp 👍 I've changed the title and description

For the response body you have a point, however I'm not sure if it should be exactly that way, because NextResponse is an extension of Response and Response doesn't take a generic, which when that changes would become a problem.

Yeah, I'm not holding my breath that TypeScript will implement generic parameters for Response, even though I'm the one that submitted the issue. If they end up doing it in some crazy future, I guess having a breaking types change to NextResponse would be reasonable...

Will discuss with the rest of the team.

Cool, would be great to get NextResponse generic parameters just at parity with the existing NextApiResponse<...> type that is available in API Routes in the pages/ directory - would be one less thing to hold back transitioning to app/ directory.


Alternative

If this could be implemented as a "recipe" or "example" in userland - with not too much code, and still being able to import + use NextResponse from next/server - this may also be acceptable... not as nice as having it built-in for everyone to use out of the box, but it may also work.

Alternative 2

Patching next using patch-package to add the type is always an option, but we're really trying to avoid maintaining custom patches for Next.js, which gets especially complex with the projects the students create.

@karlhorky karlhorky changed the title Accept generic type parameters for NextResponse and NextRequest Accept generic type parameters for NextResponse Mar 5, 2023
@karlhorky
Copy link
Contributor Author

karlhorky commented Mar 5, 2023

I had a quick look into it and the change would be something like this:

<code block>

Seems like this approach is working mostly for our app code too - copied your update into node_modules/next/dist/server/web/spec-extension/response.d.ts.

For example, both of the responses are checked properly here, when using the return type on the GET Route Handler:

type ResponseBody = { errors: { message: string }[] } | { username: string };

export async function GET(
  request: NextRequest,
): Promise<NextResponse<ResponseBody>> {
  const token = request.cookies.get('sessionToken');
  const user = !token?.value
    ? undefined
    : await getUserByValidSessionToken(token.value);

  if (!user) {
    return NextResponse.json({
      errors: [{ message: 'User not found by session token' }],
    });
  }

  return NextResponse.json({
    username: user.username,
  });
}
Kapture.2023-03-05.at.16.08.12.mp4

Caveat

No excess property checks: One thing you'll notice at the very end of the video above is that the extra a property added in the response does not result in an error.

@karlhorky
Copy link
Contributor Author

karlhorky commented Mar 25, 2023

@timneutkens I opened a PR here to continue the discussion:

I also included these changes to the code in your comment above:

  • I gave the generic parameters better names (not single characters)
  • I changed the default value of the generic body parameter from void to unknown, so that the generic argument does not need to be specified

@karlhorky
Copy link
Contributor Author

karlhorky commented Apr 8, 2023

Workaround

In the meantime, until the PR above gets reviewed, there is a 2nd workaround without patching next of adding a new declaration file for Next.js. It's a bit messy and it will need to be maintained with any updates that are made to the next package, but it works.

It can be done by adding a next.d.ts file and then updating the "includes" array in your tsconfig.json file to include this file:

next.d.ts

import { NextResponse } from 'next/server';

declare const INTERNALS: unique symbol;

declare module 'next/server' {
  /**
   * Workaround to enable generic type arguments for NextResponse until
   * PR is merged:
   * https://github.com/vercel/next.js/pull/47526
   * Original issue: https://github.com/vercel/next.js/issues/45943
   *
   * Eg.
   *
   * ```ts
   * type ResponseBodyGet = { a: number };
   * export function GET(): NextResponse<ResponseBodyGet> {
   *   if (Math.random() > 0.5) {
   *     return NextResponse.json({ invalid: 'x' }) // ❌ Type 'NextResponse<{ invalid: string; }>' is not assignable to type 'NextResponse<ResponseBodyGet>'.
   *   }
   *   return NextResponse.json({ a: 1 }); // ✅ No error
   * }
   * ```
   */
  export class NextResponse<Body = unknown> extends Response {
    [INTERNALS]: {
      cookies: ResponseCookies;
      url?: NextURL;
      Body?: Body;
    };
    static json<JsonBody>(
      body: JsonBody,
      init?: ResponseInit,
    ): NextResponse<JsonBody>;
  }
}

tsconfig.json

{
  "include": [
    "next-env.d.ts",
+   "next.d.ts",
    "**/*.ts",
    "**/*.tsx",
    ".next/types/**/*.ts"
  ]
}

Because of declaration merging, this will use the updated type declarations for:

  1. NextResponse to allow generic type arguments
  2. NextResponse.json() to check against the generic type argument passed

But will still use the types from next/server for anything else eg. NextResponse.redirect()

karlhorky added a commit to upleveled/eslint-config-upleveled that referenced this issue Apr 8, 2023
shuding added a commit that referenced this issue May 19, 2023
Fixes #45943

### What?

Allow for type checking the return type of a Route Handler in the `app/`
directory:

[TypeScript
Playground](https://www.typescriptlang.org/play?#code/MYewdgzgLgBAkgOQCoFEBKCCCAZAyjAXhlwE8BbAIxABsAKAcgEswoBTAJzAENqZ3WIAB3ARW9AJQAoSawAew9rGDUuECDARyoaAcMisAPACEQAExKEYAVzABrMCADuYAHwwtrMKfU6hI1jAA3pIwMADaiKgYOLgAugBcQSGhMCbmAPyJaSQANMkAvtKhoJBQ7FbAUCDstFQZWWYkcGCMsAA+MGBW1NQ5MMytib56os2tloH54kkpMBBWghy1jX0DUFLJoVAAFowQEcjoWHixE4WhhcnQXFCMwDAAVhDgBgBSz2DZLsvmie-g2VWLSgmRgw38Y3WiU0sm0un8bw+XxmKQA9KiYAABKAQAC0ckWlXx7HY1RgSF26j2nRAsGYMGojAoqNMIDIfEYAHNtrAHI4+lwvDBHAFgIL6LAuFZOWRPHSoAA6TYwErQPjw-RDDWiSzg-QKp7gH65frAqSzfhQKycTqsRwaLR60S0fh+fV1E2ukasc0wQqXAnVWAAMxslUY4BgAHEUEhaOJoY7tYZAlwKMBEgBGABMAGZ8m5AjB0dHPBw7jAuOxOVZZSwYIJVKJTMlGMGYLQALI3bYK9iC1lkeMwNwABgVAFZpkXlZbrWAHbCnawDR9aKn01m81NixjAKDkNPcJOqiQ9Kuq-EqBWSc5tMLhbtEq6NgVMrGDiXosm-9B3JYPHCkuwp6NKaoAkqwV4BvIQYwKGYDhpGMZINm8aJkuyZBLuGggDAnJluwFZVjWdawFYzatu2XY9n2A5ssOY6TtOMCzqwVp3kmj4roaYDrmmGYwDm+bTP+h6ASelZgBYZ5cMAwCsIIbAthcN5sfOi4Pt6z68a+76ft+si-iJ+5icewGSdJoGyfJimsC2hRAA)

![Screenshot 2023-03-25 at 16 59
51](https://user-images.githubusercontent.com/1935696/227728328-43350f50-5067-4d27-8701-000569e757cc.png)

### Why?

There is currently no nice way to enforce the return type of a Route
Handler in the `app/` directory.

This is possible with `NextApiResponse` in the `pages/` directory,
because of this PR:

- #7841

### How?

This implements a generic parameter for the `NextResponse` class, which
can be used to pass a type argument when setting the return type for a
Route Handler.

<!-- Thanks for opening a PR! Your contribution is much appreciated.
To make sure your PR is handled as smoothly as possible we request that
you follow the checklist sections below.
Choose the right checklist for the change(s) that you're making:

## For Contributors

### Improving Documentation or adding/fixing Examples

- The "examples guidelines" are followed from our contributing doc
https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md
- Make sure the linting passes by running `pnpm build && pnpm lint`. See
https://github.com/vercel/next.js/blob/canary/contributing/repository/linting.md

### Fixing a bug

- Related issues linked using `fixes #number`
- Tests added. See:
https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs
- Errors have a helpful link attached, see
https://github.com/vercel/next.js/blob/canary/contributing.md

### Adding a feature

- Implements an existing feature request or RFC. Make sure the feature
request has been accepted for implementation before opening a PR. (A
discussion must be opened, see
https://github.com/vercel/next.js/discussions/new?category=ideas)
- Related issues/discussions are linked using `fixes #number`
- e2e tests added
(https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs
- Documentation added
- Telemetry added. In case of a feature if it's used or not.
- Errors have a helpful link attached, see
https://github.com/vercel/next.js/blob/canary/contributing.md



## For Maintainers

- Minimal description (aim for explaining to someone not on the team to
understand the PR)
- When linking to a Slack thread, you might want to share details of the
conclusion
- Link both the Linear (Fixes NEXT-xxx) and the GitHub issues
- Add review comments if necessary to explain to the reviewer the logic
behind a change

### What?

### Why?

### How?

Closes NEXT-
Fixes #

-->

---------

Co-authored-by: Shu Ding <g@shud.in>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
hydRAnger pushed a commit to hydRAnger/next.js that referenced this issue Jun 12, 2023
Fixes vercel#45943

### What?

Allow for type checking the return type of a Route Handler in the `app/`
directory:

[TypeScript
Playground](https://www.typescriptlang.org/play?#code/MYewdgzgLgBAkgOQCoFEBKCCCAZAyjAXhlwE8BbAIxABsAKAcgEswoBTAJzAENqZ3WIAB3ARW9AJQAoSawAew9rGDUuECDARyoaAcMisAPACEQAExKEYAVzABrMCADuYAHwwtrMKfU6hI1jAA3pIwMADaiKgYOLgAugBcQSGhMCbmAPyJaSQANMkAvtKhoJBQ7FbAUCDstFQZWWYkcGCMsAA+MGBW1NQ5MMytib56os2tloH54kkpMBBWghy1jX0DUFLJoVAAFowQEcjoWHixE4WhhcnQXFCMwDAAVhDgBgBSz2DZLsvmie-g2VWLSgmRgw38Y3WiU0sm0un8bw+XxmKQA9KiYAABKAQAC0ckWlXx7HY1RgSF26j2nRAsGYMGojAoqNMIDIfEYAHNtrAHI4+lwvDBHAFgIL6LAuFZOWRPHSoAA6TYwErQPjw-RDDWiSzg-QKp7gH65frAqSzfhQKycTqsRwaLR60S0fh+fV1E2ukasc0wQqXAnVWAAMxslUY4BgAHEUEhaOJoY7tYZAlwKMBEgBGABMAGZ8m5AjB0dHPBw7jAuOxOVZZSwYIJVKJTMlGMGYLQALI3bYK9iC1lkeMwNwABgVAFZpkXlZbrWAHbCnawDR9aKn01m81NixjAKDkNPcJOqiQ9Kuq-EqBWSc5tMLhbtEq6NgVMrGDiXosm-9B3JYPHCkuwp6NKaoAkqwV4BvIQYwKGYDhpGMZINm8aJkuyZBLuGggDAnJluwFZVjWdawFYzatu2XY9n2A5ssOY6TtOMCzqwVp3kmj4roaYDrmmGYwDm+bTP+h6ASelZgBYZ5cMAwCsIIbAthcN5sfOi4Pt6z68a+76ft+si-iJ+5icewGSdJoGyfJimsC2hRAA)

![Screenshot 2023-03-25 at 16 59
51](https://user-images.githubusercontent.com/1935696/227728328-43350f50-5067-4d27-8701-000569e757cc.png)

### Why?

There is currently no nice way to enforce the return type of a Route
Handler in the `app/` directory.

This is possible with `NextApiResponse` in the `pages/` directory,
because of this PR:

- vercel#7841

### How?

This implements a generic parameter for the `NextResponse` class, which
can be used to pass a type argument when setting the return type for a
Route Handler.

<!-- Thanks for opening a PR! Your contribution is much appreciated.
To make sure your PR is handled as smoothly as possible we request that
you follow the checklist sections below.
Choose the right checklist for the change(s) that you're making:

## For Contributors

### Improving Documentation or adding/fixing Examples

- The "examples guidelines" are followed from our contributing doc
https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md
- Make sure the linting passes by running `pnpm build && pnpm lint`. See
https://github.com/vercel/next.js/blob/canary/contributing/repository/linting.md

### Fixing a bug

- Related issues linked using `fixes #number`
- Tests added. See:
https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs
- Errors have a helpful link attached, see
https://github.com/vercel/next.js/blob/canary/contributing.md

### Adding a feature

- Implements an existing feature request or RFC. Make sure the feature
request has been accepted for implementation before opening a PR. (A
discussion must be opened, see
https://github.com/vercel/next.js/discussions/new?category=ideas)
- Related issues/discussions are linked using `fixes #number`
- e2e tests added
(https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs
- Documentation added
- Telemetry added. In case of a feature if it's used or not.
- Errors have a helpful link attached, see
https://github.com/vercel/next.js/blob/canary/contributing.md



## For Maintainers

- Minimal description (aim for explaining to someone not on the team to
understand the PR)
- When linking to a Slack thread, you might want to share details of the
conclusion
- Link both the Linear (Fixes NEXT-xxx) and the GitHub issues
- Add review comments if necessary to explain to the reviewer the logic
behind a change

### What?

### Why?

### How?

Closes NEXT-
Fixes #

-->

---------

Co-authored-by: Shu Ding <g@shud.in>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
@github-actions
Copy link
Contributor

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
TypeScript Related to types with Next.js.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants