fix: point this to parent at onRegister#5675
Conversation
|
citgm errors are irrelevant, it's not breaking any core plugins |
|
@mcollina if there is a bug here ( |
Signed-off-by: Gürgün Dayıoğlu <hey@gurgun.day>
Eomm
left a comment
There was a problem hiding this comment.
what this points to is documented neither in types nor the docs
I created a task here #4967 to remind that.
Binding this to old seems reasonable because we are saying: this is the object that called the register method, and this statement is equally true for the other hooks.
I must underline that this PR is a step forward, but if I'm not wrong, to close the issue we apply the
boundThisAndParamsObject.call(instance, { options, instance }) interface as shared here: #4967 (comment)
|
Just for context, for v5, Matteo pointed out it’s too late to make a big breaking change, so I think we’ll defer the parameter change to v6, but we should at least have consistent this bindings in v5, and luckily this one doesn’t seem to break any plugins and achieves that - probably because it wasn’t even documented |
Uzlopak
left a comment
There was a problem hiding this comment.
RSLGTM
Makes actually sense, i guess
|
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. |
For #4967
To onRegister hooks we pass
thisand the instance, however there are 2 problems:thispoints to is documented neither in types nor the docsinstance, is the new fastify instance, butthisactually points to the avvio instance as can be seen from here:fastify/fastify.js
Line 446 in 501be89
This PR passes the parent as
thisFor more info, see: #5670 (comment)