Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

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

Merged
mcollina merged 4 commits into
fastify:masterfastify/fastify:masterfrom
jineshshah36:masterCopy head branch name to clipboard
Sep 17, 2018
Merged

added generic Query, Params, Headers, and Body types#1160
mcollina merged 4 commits into
fastify:masterfastify/fastify:masterfrom
jineshshah36:masterCopy head branch name to clipboard

Conversation

@jineshshah36

@jineshshah36 jineshshah36 commented Sep 14, 2018

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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
Copy Markdown
Member

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

@jineshshah36

Copy link
Copy Markdown
Contributor Author

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

@allevo allevo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice work!

LGTM

@mcollina mcollina left a comment

Copy link
Copy Markdown
Member

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
Copy Markdown

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.

4 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.