Add this types to decorator functions#3203
Conversation
climba03003
left a comment
There was a problem hiding this comment.
I think any of the type merge with any will become any.
test/types/instance.test-d.ts
Outdated
| expectType<string>(server.printRoutes()) | ||
|
|
||
| server.decorate('test', function (x: string) { | ||
| expectType<FastifyInstance | any>(this) |
There was a problem hiding this comment.
If it is FastifyInstance | any, I think it would become any at the end?
There was a problem hiding this comment.
I think you're right. But then the tests are actually useless, aren't they?
There was a problem hiding this comment.
We can't do expectType<FastifyRequest>(this)?
There was a problem hiding this comment.
For me, the test then fails. I guess the problem is that the type of the decorator value is types as any.
decorate(property: string | symbol, value: any, dependencies?: string[]): FastifyInstance<RawServer, RawRequest, RawReply, Logger>;There was a problem hiding this comment.
AFAIK, any overload on top of any the type will be merged to becomes any. So, either change the any to something generic without Function or this PR should be no actual gain.
|
I have added generics to the 3 decorator functions. This can now be used to check whether the value to be decorated is a function, and if so, the correct this type is added. const server = fastify()
server.decorate<(n: number) => void>('myDecorator', function(n: number): void {
this // has FastifyInstance type now
}) |
There was a problem hiding this comment.
I like the idea of Generic types for solving this issue. Can you add an test-case for not specify the types? Generic suppose to works without explicit specification.
For example the below test should works.
server.decorate('test', function (x: string): void {
expectType<FastifyInstance>(this)
})
climba03003
left a comment
There was a problem hiding this comment.
Just one more thing. Add both test case for decorateRequest and decorateReply without Generic specification. Then it is all good.
Just to ensure three of them can use with or without explicit specification.
|
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. |
Checklist
npm run testandnpm run benchmarkand the Code of conduct
Just added the correct type for the
thisbinding in the decorator functions. Additionally I added a note in the documentation that an arrow function in thedecoratefunction would break the this binding.