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
refactor(types): deduplicate listen
options and export it
#4013
Conversation
import { FastifyError } from '@fastify/error' | ||
import { ConstraintStrategy, HTTPVersion } from 'find-my-way' | ||
import * as http from 'http' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was moved due to VSCode's auto-import, should I leave it as-is, or move it back to the top? 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am ok with this change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
I'm so sorry, I didn't merge straight away, and now this conflicts. Could you rebase? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
ff96205
to
1d5e74c
Compare
No worries, I have just rebased the PR :) Edit: I made a second force push to add a missing space from the original code. |
1d5e74c
to
c56d4c8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
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. |
The options interface for
FastifyInstance.listen
was duplicated, which could potentially lead to type signature and documentation differences between the two instances. To solve this, I have made a new interface to unify the two, so there's only one source of truth for both overloads.I also added
export
to it because I found it useful - I maintain a library that uses Fastify and it allows users to configure it by passing an object with all the options, which is one of the supported (and non-deprecated) overloads of thelisten
function. Currently, we would need a very hacky type to get said options from the overloads.Click to see the code
So for ease of development, I think this export would be beneficial, not for us, but quite possibly for a lot of people as well.
Checklist
npm run test
andnpm run benchmark
and the Code of conduct