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

Cannot use type on app router API route handler request: Request #1467

Closed
6 tasks done
michaeloliverx opened this issue Oct 2, 2023 · 7 comments · Fixed by #1476
Closed
6 tasks done

Cannot use type on app router API route handler request: Request #1467

michaeloliverx opened this issue Oct 2, 2023 · 7 comments · Fixed by #1476
Labels
question Further information is requested

Comments

@michaeloliverx
Copy link

Checklist

Description

Next.js 13 supports using Request and Response types, Auth0 SDK complains about types.

Reproduction

import { withApiAuthRequired, getAccessToken } from "@auth0/nextjs-auth0/edge";

const GET = withApiAuthRequired(async (request: Request) => {
  const { accessToken } = await getAccessToken({refresh: true});

  return Response.json({ accessToken });
});

export { GET };

export const runtime = "edge";
image

Additional context

No response

nextjs-auth0 version

3.1.0

Next.js version

13.5.3

Node.js version

16.19.1

@adamjmcgrath
Copy link
Contributor

Hi @michaeloliverx - thanks for raising this

The request object is a NextRequest (see https://nextjs.org/docs/app/api-reference/file-conventions/route#parameters) (The SDK relies on this because it uses the cookies helper of NextRequest)

What's the problem with you using NextRequest as the type?

@adamjmcgrath adamjmcgrath added the question Further information is requested label Oct 2, 2023
@michaeloliverx
Copy link
Author

Thanks for the fast response!

The SDK relies on this because it uses the cookies helper of NextRequest)

I wonder if the SDK used the cookies function would it mean user code could use either type?

import { cookies } from 'next/headers'

...

const cookieStore = cookies()

What's the problem with you using NextRequest as the type?

Nothing really I just wasn't aware what the problem was and usually I prefer to use the native Request and Response globals.

I guess a note in the docs might help others as well?

@adamjmcgrath
Copy link
Contributor

I wonder if the SDK used the cookies function would it mean user code could use either type?

Even if the SDK didn't use req.cookies, the Next.js route accepts a NextRequest whether you type it as a Request or not.

I guess a note in the docs might help others as well?

I don't think a special callout is required for this SDK just accurately typing a Next api route

@michaeloliverx
Copy link
Author

michaeloliverx commented Oct 2, 2023

Even if the SDK didn't use req.cookies, the Next.js route accepts a NextRequest whether you type it as a Request or not.

I totally get what you are saying and using NextRequest would be more "correct" but the Next.js docs show examples using the Request type and returning a new Response instance.

The cookies and headers docs show examples for both Request and NextRequest but the Request Body example only shows using Request. In fact the docs are littered with examples that only refer to the native fetch primitives Request/Response. I think this can be confusing for users of this SDK (it was for me).


Also if you look at the streaming section on the same page, that example returns a StreamingTextResponse instance to stream LLM model responses.
That class is typed as a extension of the native Response object:

/**
 * A utility class for streaming text responses.
 */
declare class StreamingTextResponse extends Response {
    constructor(res: ReadableStream, init?: ResponseInit, data?: experimental_StreamData);
}

I am personally using the Vercel AI SDK (https://github.com/vercel/ai) so after changing the request type to NextRequest I still get a type error

import { withApiAuthRequired } from "@auth0/nextjs-auth0/edge";
import OpenAI from "openai";
import { type NextRequest } from "next/server";
import { OpenAIStream, StreamingTextResponse } from "ai";

export const runtime = "edge";

const openai = new OpenAI({
  apiKey: process.env.OPENAI_API_KEY,
});

export const GET = withApiAuthRequired(async (request: NextRequest) => {
  const { messages } = await request.json();

  const response = await openai.chat.completions.create({
    model: "gpt-4-0613",
    stream: true,
    messages: [...messages],
  });

  const stream = OpenAIStream(response, {});

  return new StreamingTextResponse(stream, {});
});

TS output:

Overload 1 of 2, '(apiRoute: AppRouteHandlerFn): AppRouteHandlerFn', gave the following error.
    Argument of type '(request: NextRequest) => Promise<StreamingTextResponse>' is not assignable to parameter of type 'AppRouteHandlerFn'.
      Type 'Promise<StreamingTextResponse>' is not assignable to type 'NextResponse<unknown> | Promise<NextResponse<unknown>>'.
        Type 'Promise<StreamingTextResponse>' is not assignable to type 'Promise<NextResponse<unknown>>'.
          Type 'StreamingTextResponse' is missing the following properties from type 'NextResponse<unknown>': cookies, [INTERNALS]

From a user and DX perspective it would be a great win if it just worked with the Request and Response.

@adamjmcgrath
Copy link
Contributor

I still think NextRequest is the right type for the request object, but I've just noticed your comment about the type of the response

Also if you look at the streaming section on the same page, that example returns a StreamingTextResponse instance to stream LLM model responses.

You are correct, the function you pass to withApiAuthRequired can return a Response (or any superset thereof) - so the return type should be Response, not NextResponse - will change that.

I am personally using the Vercel AI SDK (vercel/ai) so after changing the request type to NextRequest I still get a type error

Can you confirm if your code snippet you've shared actually works? (if you ignore the type error) I haven't tested this with a StreamingTextResponse, and I'm not sure if the way I'm cloning a Request will suffice for this use case

@michaeloliverx
Copy link
Author

michaeloliverx commented Oct 3, 2023

Can you confirm if your code snippet you've shared actually works? (if you ignore the type error) I haven't tested this with a StreamingTextResponse, and I'm not sure if the way I'm cloning a Request will suffice for this use case

The snippet I shared isn't the exact code I am running as I stripped out all the logic.. BUT it does seem to work yes!

Hitting it without auth gives me a 401 with

{
    "error": "not_authenticated",
    "description": "The user does not have an active session or is not authenticated",
}

And hitting it with does stream the text fine.

@adamjmcgrath
Copy link
Contributor

Great - thanks for confirming @michaeloliverx

Will follow up with a PR shortly

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

Successfully merging a pull request may close this issue.

2 participants