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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support custom Request types #189

Closed
2 tasks done
wilkmaia opened this issue May 3, 2022 · 2 comments 路 Fixed by #190
Closed
2 tasks done

Support custom Request types #189

wilkmaia opened this issue May 3, 2022 · 2 comments 路 Fixed by #190

Comments

@wilkmaia
Copy link
Contributor

wilkmaia commented May 3, 2022

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure the feature has not already been requested

馃殌 Feature Proposal

Support for the request object (instance of the Request (lib/request.js) class) to inherit from a custom class besides stream.Readable.

Motivation

Starting from v12.0.9, next.js validates the request object's type is an http.IncomingMessage instance. If it's not, no error is thrown but the request will fail due to an internal type conversion not being made.

This is an issue for using light-my-request to test API requests that use next.js. Specifically, this is currently blocking bumping next.js' version in fastify-nextjs (fastify/fastify-nextjs#514) because tests don't pass with the current structure of light-my-request.

Example

Supporting a new options flag, namely customRequestType and adding the following snippet in lib/request.js:124 (Request function definition) should do the trick:

  if (options.customRequestType) {
    util.inherits(this.constructor, options.customRequestType)
  }
@mcollina
Copy link
Member

mcollina commented May 3, 2022

When did Next.js make this change? I think it would be better to approach them and see if it could be relaxed somehow.

Making it pluggable would be impossible as there is specific behavior that is needed - and it's not possible to bolt it on top of another prototype. This library does not use IncomingMessage because it'd be very hard to do so. It's not impossible, so you might give that a try.

@wilkmaia
Copy link
Contributor Author

wilkmaia commented May 3, 2022

@mcollina in https://github.com/vercel/next.js/releases/tag/v12.0.9.

Specifically in this commit.

The issue is that before they simply set the TypeScript request type to http.IncomingRequest (e.g. packages/next/server/base-server.ts:367). In that commit they started using the BaseNextRequest type for the request object in the base-server file and moved the front-end implementation to packages/next/server/next-server.ts, adding a conversion from http.IncomingMessage to BaseNextRequest with the normalizeReq function.

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

Successfully merging a pull request may close this issue.

2 participants