Improve NextConnect method Types

[saucesteals]

A small (yet possibly breaking) change of using own generic type arguments as the sole types for Req and Res in RequestHandler and Middleware While the syntax of Req & ReqExt does allow for new properties to get introduced to the request (or response), it does not allow for "modifying" the ones in Req/Res

We can preserve the fact that the Req and Res types will always extend the types passed into nc while also allowing a sort of "modification" (no idea what the official term for this is if there even is one)

I know this can be achieved by simply getting rid of the types in the original nc<Req> Req type that u would ever want to add on later as another type, but I think this is simply a better approach to this (sidenote: it is also how express handles it so I may have a little bias from using it)

Example usage with asserting yup schemas:

interface CustomRequest<T = any> extends NextApiRequest {
  body: T;
}

const validateData = <T extends yup.ObjectSchema<any>>(schema: T) => {
  return async (req: CustomRequest<yup.Asserts<T>>) => {
    // ...
  };
};

const handler = nc<CustomRequest, NextApiResponse>();

handler.post(
  "/path",
  validateData(yup.object({ foo: yup.string().required() })),
  (req) => {
    req.body.foo;
    // (property) foo: string   <------
  }
);

[changeset-bot[bot]]

⚠️ No Changeset found

Latest commit: 09aff637c8b84e90a1ba21e2b38fba630f23ea44

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.

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

[codecov[bot]]

Codecov Report

Merging #171 (09aff63) into master (14aa547) will not change coverage. The diff coverage is n/a.

@@            Coverage Diff            @@
##            master      #171   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines           81        81           
=========================================
  Hits            81        81           

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 14aa547...09aff63. Read the comment docs.

[hoangvvo]

This is indeed a breaking change for sure. Unfortunately, it might not be possible to be adopted at this point even though I agree it might look better. With this new change, I believe the current syntax of extending will no longer work:

interface ExtendedRequest {
  user: string;
}
interface ExtendedResponse {
  cookie(name: string, value: string): void;
}

handler.post<ExtendedRequest, ExtendedResponse>((req, res) => {  // <-- this will probably fail since ExtendedRequest/Response does not extend IncomingMessage/...
  req.user = "Anakin";
  res.cookie("sid", "8108");
});

[saucesteals]

If the request and response types do not extend the ones passed into nc<Req,Res>() it will definitely error and would instead require:

interface ExtendedRequest extends IncomingMessage {
  user: string;
}
interface ExtendedResponse extends ServerResponse {
  cookie(name: string, value: string): void;
}

This does seem better to me personally even without considering the new functionality it may allow for, but I do agree that a change this small and specific might not be enough for an entire new breaking version.

[Alannek1992]

Hi, I am also dealing with this issue. What about this suggestion:

Current impl of the post function: post<ReqExt = {}, ResExt = {}>( ...handlers: RequestHandler<Req & ReqExt, Res & ResExt>[] ): this; My proposal: post<ReqExt extends Req = {}, ResExt extends Res = {}>( ...handlers: RequestHandler<ReqExt, ResExt>[] ): this; I am describing the issue in my words here: #172

[hoangvvo]

Ultimately I think this change, while great, does not fit my requirement at the moment. Since having to create a new interface extending the base for every single route might be a bit inconvenient and breaking.

A better solution for the typing of things like req.body would come from Next.js itself by allowing generic like TBody in NextAPIRequest (in which case we would set it to unknown for the base, allowing us to manually set it to anything later). For now, I guess the only way is to omit body from the NextAPIRequest for the base.

[ravinggenius]

@hoangvvo at first I didn't like your response, but then I thought about it more. Turns out it's a somewhat common request for Next. As explained in https://github.com/vercel/next.js/pull/15139#issuecomment-657866925, req.body (and URL query strings et cetera) are direct user input. They could literally be anything, and adding specific types for those data doesn't change that. User data should never be trusted at face value. I recently discovered zod, which can perform a runtime validation on incoming data and return a strongly-typed object (if pass) or throw an error (if fail). I believe that's a much better solution. Other libraries are likely able to provide similar functionality.

On the other hand, NextApiResponse does allow for a generic typing of the response body. Maybe you could allow for a method-specific type argument to be passed on to NextApiResponse? This could help keep us developers honest with the data we're responding with.

---

Edit: specifically something like:

const handler = nc().get<{ answer: number; }>((req, res) => {
  // works
  res.json({ answer: 42 });

  // type error
  res.json({ answer: "what?" });
});