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.

Sign up for GitHub

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

Jump to bottom

Add optional generic parameter to NextResponse #47526

Merged
merged 10 commits into from
May 19, 2023
Merged

Add optional generic parameter to NextResponse #47526

Changes from 4 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
feb0cd6
Add optional generic parameter to NextResponse
karlhorky Mar 25, 2023
67301b3
Fix formatting
karlhorky Mar 25, 2023
62d788a
Fix formatting
karlhorky Mar 25, 2023
4350c28
Merge branch 'canary' into patch-3
karlhorky May 11, 2023
5b73170
Merge branch 'canary' into patch-3
karlhorky May 17, 2023
3f59852
Lowercase body
karlhorky May 18, 2023
b037fd0
Merge branch 'canary' into patch-3
karlhorky May 18, 2023
7c9f3cd
Merge branch 'canary' into patch-3
shuding May 19, 2023
29ca085
Merge branch 'canary' into patch-3
kodiakhq[bot] May 19, 2023
5bcc8b1
Merge branch 'canary' into patch-3
kodiakhq[bot] May 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 6 additions & 2 deletions packages/next/src/server/web/spec-extension/response.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,11 @@ function handleMiddlewareField(
}
}

export class NextResponse extends Response {
export class NextResponse<Body = unknown> extends Response {
[INTERNALS]: {
cookies: ResponseCookies
url?: NextURL
Body?: Body
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems redundant?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that's what I thought at first too, but [INTERNALS] is apparently important to allow for checking against the consequent executions of the .json() method, check out the line with the ❌ below (it should be an error but it is not)

TypeScript Playground

Screenshot 2023-05-17 at 22 27 05

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should that be body?: Body then (lowercased)?

Copy link
Contributor Author

@karlhorky karlhorky May 18, 2023 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm ok with making this change, seems to be consistent with the other types in [INTERNALS]

still works:

TypeScript Playground

Screenshot 2023-05-18 at 17 48 31

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 3f59852

}

constructor(body?: BodyInit | null, init: ResponseInit = {}) {
Expand Down Expand Up @@ -66,7 +67,10 @@ export class NextResponse extends Response {
return this[INTERNALS].cookies
}

static json(body: any, init?: ResponseInit): NextResponse {
static json<JsonBody>(
body: JsonBody,
init?: ResponseInit
): NextResponse<JsonBody> {
// @ts-expect-error This is not in lib/dom right now, and we can't augment it.
const response: Response = Response.json(body, init)
return new NextResponse(response.body, response)
Expand Down