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

TypeScript RouteOptions interface generics missing defaults #1667

Closed
fox1t opened this issue May 23, 2019 · 2 comments 路 Fixed by #1669
Closed

TypeScript RouteOptions interface generics missing defaults #1667

fox1t opened this issue May 23, 2019 · 2 comments 路 Fixed by #1669
Labels
typescript TypeScript related v2.x Issue or pr related to Fastify v2

Comments

@fox1t
Copy link
Member

fox1t commented May 23, 2019

馃悰 Bug Report

Working with routes definition I found that RouteOptions interface hasn't default generics declared. (https://github.com/fastify/fastify/blob/master/fastify.d.ts#L266) as opposed to RouteShorthandOptions(https://github.com/fastify/fastify/blob/master/fastify.d.ts#L230) that it extends from.
Is this a wanted behavior? I think this is too verbose since it forces to pass every time a route is declared a full set of same generics: RouteOptions<Server, IncomingMessage, ServerResponse>.

To Reproduce

Removing generics (<Server, IncomingMessage, ServerResponse>) TypeScript pops a compiler error:
Generic type 'RouteOptions<HttpServer, HttpRequest, HttpResponse, Query, Params, Headers, Body>' requires between 3 and 7 type arguments.

import { Server, IncomingMessage, ServerResponse } from 'http'
import { RouteOptions, RouteShorthandOptions } from 'fastify'

const listAttendance: RouteOptions<Server, IncomingMessage, ServerResponse> = {
  method: 'GET',
  url: '/',
  handler: (req, reply) => {
    reply.send([])
  },
}

const listAttendances: RouteShorthandOptions = {}

Expected behavior

Adding the defaults to RouteOptions the error goes away. I think this is the desirable behavior.

Paste the results here:

/**
   * Route configuration options such as "url" and "method"
   */
  interface RouteOptions<
    HttpServer = http.Server,
    HttpRequest = http.IncomingMessage,
    HttpResponse = http.ServerResponse,
    Query = DefaultQuery,
    Params = DefaultParams,
    Headers = DefaultHeaders,
    Body = DefaultBody
  >
    extends RouteShorthandOptions<
      HttpServer,
      HttpRequest,
      HttpResponse,
      Query,
      Params,
      Headers,
      Body
    > {
    method: HTTPMethod | HTTPMethod[]
    url: string
    handler: RequestHandler<HttpRequest, HttpResponse, Query, Params, Headers, Body>
  }

Your Environment

  • node version: 10
  • fastify version: 2.4.1
  • os: Mac
@Ethan-Arrowood
Copy link
Member

Yes this change would be valid (in fact it is almost identical to the upcoming v3 types just using different generics). Would you like to send a pr to update this on master?

@Ethan-Arrowood Ethan-Arrowood added typescript TypeScript related v2.x Issue or pr related to Fastify v2 semver-minor Issue or PR that should land as semver minor labels May 23, 2019
@Ethan-Arrowood Ethan-Arrowood removed the semver-minor Issue or PR that should land as semver minor label May 23, 2019
@fox1t
Copy link
Member Author

fox1t commented May 23, 2019

Done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
typescript TypeScript related v2.x Issue or pr related to Fastify v2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants