types: extended FastifyAuthFunction type to accept async functions
Checklist
- [ ] run
npm run testandnpm run benchmark - [x] tests and/or benchmarks are included
- [ ] documentation is changed or added
- [x] commit message and code follows the Developer's Certification of Origin and the Code of conduct
All tests are passing:
- [x] Unit tests
- [x] TypeScript tests
- Benchmark script not included, so I couldn't run.
- Documentation is not changed because of this change is only affect TypeScript type annotation, we can add if needed.
I would like to open a PR for this issue due to contribute if you allow me. I've made the the update and updated the tests in this context. Due to TypeScript's inability to automatically infer 'this' context in union types, needed to explicitly specify 'this: FastifyInstance' type annotation in functions that use 'this'. I've updated the tests to reflect this requirement, also added async and promise-based tests to cover the update.
In the function below, which is used without specifying a pre-usable type, the need to use 'this: FastifyInstance' arises due to the situation I mentioned above.
app.auth([
// this should be transform to function (this: FastifyInstance) for TypeScript annotation needs.
function () {
expectType<FastifyInstance>(this)
return Promise.resolve()
},
])
Changes made:
- Updated FastifyAuthFunction type to support both callback and Promise patterns with proper this context
- Added explicit this type annotation in auth tests
- Reorganized test structure for better readability
- tests with { relation: 'or' } added, defaultRelation is already 'or' but defaultRelation definition can be changed.
Please review the changes.
Afaik this is not working properly. Basically you get a "merged" function. Basically
((this: FastifyInstance, request: Request, reply: Reply, done: (error?: Error) => void) => void)
| ((this: FastifyInstance, request: Request, reply: Reply) => Promise<void>)
results in
((this: FastifyInstance, request: Request, reply: Reply, done?: (error?: Error) => void) => (void | Promise<void)
compare with https://github.com/fastify/fastify/blob/be8f72fcb3c99fad1982fe4853119a9a79db40ba/types/hooks.d.ts#L78
Thanks for your comment @Uzlopak , I updated the PR after reviewing the hook link you mentioned and now it looks better and meets the issues mentioned above.