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

added generic Query, Params, Headers, and Body types #1160

Merged
merged 4 commits into from
Sep 17, 2018
Merged

added generic Query, Params, Headers, and Body types #1160

merged 4 commits into from
Sep 17, 2018

Conversation

jineshshah36
Copy link
Contributor

@jineshshah36 jineshshah36 commented Sep 14, 2018

Checklist

  • run npm run test and npm run benchmark
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message and code follows Code of conduct

First off, this is a completely non-breaking change for typescript users. The main change here is that I converted the query, params, headers, and body types into generics that default to the existing type definitions. Since we can set the schema validators, I think it makes sense to also allow us to exactly type the pieces that can be validated since we will know exactly what their types should be. However, this should not be required which is why I defaulted these new generic types to their existing values, making this feature completely opt-in for typescript users.

Right now, overriding one of the generic types will be slightly verbose since typescript cannot infer generic parameters when one is overridden out of order, but generic parameter type inference is already on their roadmap for 3.2 (microsoft/TypeScript#26349). My opinion is that this should be merged now since it isn't breaking, and once 3.2 comes out, it will only be easier.

@jineshshah36 jineshshah36 changed the title added default Query, Params, Headers, and Body types added generic Query, Params, Headers, and Body types Sep 14, 2018
@jineshshah36
Copy link
Contributor Author

As an example how I can use this in a real-world example:

type Query = {
  foo: string
  bar: number
}

type Params = {
  foo: string
}

type Headers = {
  "X-Access-Token": string
}

type Body = {
  foo: {
    bar: {
      baz: number
    }
  }
}

server.get<Query, Params, Headers, Body>("/", ({ query, params, headers, body }, reply) => {
  // ...
})

However, they are all optional, so I could also do the following:

import { DefaultQuery as Q, DefaultParams as P, DefaultBody as B } from 'fastify'

type Headers = {
  "X-Access-Token": string
}

server.get<Q, P, Headers, B>("/", ({ query, params, headers, body }, reply) => {
  // ...
})

And finally, as it's non-breaking, this also works and automatically infers defaults:

server.get("/", ({ query, params, headers, body }, reply) => {
  // ...
})

@mcollina
Copy link
Member

Good work! Can you add that example to our tests as well?

@jineshshah36
Copy link
Contributor Author

I added tests and a section in the typescript docs explaining how to use it.

Copy link
Member

@allevo allevo left a comment

Choose a reason for hiding this comment

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

Nice work!

LGTM

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

LGTM

@mcollina mcollina merged commit 15fbe24 into fastify:master Sep 17, 2018
@delvedor delvedor added the typescript TypeScript related label Sep 17, 2018
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
typescript TypeScript related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants