Skip to content

Conversation

@brunozoric
Copy link
Contributor

@brunozoric brunozoric commented Aug 8, 2022

The type in the enforceBase64 callable is not of FastifyReply type but Response from light-my-request package.
I changed code to note that the parameter is a response, not a reply.
Also changed reply to response in the explanation of the enforceBase64 parameter in the docs.

Checklist

@Uzlopak
Copy link
Contributor

Uzlopak commented Aug 8, 2022

Why should it be a light my request response?

@brunozoric
Copy link
Contributor Author

You can see it by the type written here.
LightMyRequestCallback is of type:

type CallbackFunc = (err: Error, response: Response) => void

So response passed into the enforceBase64 callable is of Response type.

@Uzlopak
Copy link
Contributor

Uzlopak commented Aug 8, 2022

If you use .inject() in your typings than you have Response. If you use it in production (not injecting) it should be FastifyReply

@brunozoric
Copy link
Contributor Author

brunozoric commented Aug 8, 2022

Code uses .inject() (check here).
And there response is passed into the customBinaryCheck() here.

@Uzlopak
Copy link
Contributor

Uzlopak commented Aug 8, 2022

Ah, makes sense.

@Uzlopak Uzlopak requested a review from adrai August 8, 2022 10:01
Copy link
Contributor

@Uzlopak Uzlopak left a comment

Choose a reason for hiding this comment

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

LGTM

@Uzlopak
Copy link
Contributor

Uzlopak commented Aug 8, 2022

@adrai
Do we need more reviewers?

@adrai
Copy link
Member

adrai commented Aug 8, 2022

@melchor629 ?

@melchor629
Copy link
Contributor

Oh, the change makes a lot of sense.

From my side, LGTM 👍

@adrai adrai merged commit a9c7c21 into fastify:master Aug 8, 2022
@adrai
Copy link
Member

adrai commented Aug 8, 2022

included in v3.1.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants