Skip to content
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

types: add FastifyJwtNamespace helper #303

Merged
merged 4 commits into from
Aug 24, 2023
Merged

types: add FastifyJwtNamespace helper #303

merged 4 commits into from
Aug 24, 2023

Conversation

Uzlopak
Copy link
Contributor

@Uzlopak Uzlopak commented Aug 10, 2023

Checklist

@Uzlopak Uzlopak requested review from climba03003, Fdawgs and a team August 10, 2023 00:37
README.md Outdated Show resolved Hide resolved
Co-authored-by: Frazer Smith <[email protected]>
@Uzlopak
Copy link
Contributor Author

Uzlopak commented Aug 10, 2023

@Fdawgs

Thanks

Copy link
Member

@Fdawgs Fdawgs left a comment

Choose a reason for hiding this comment

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

LGTM on readme.

secretFnBufferCallback: (_req, _token, cb) => { cb(null, Buffer.from('some secret', 'base64')) },
secretFnBufferPromise: (_req, _token) => Promise.resolve(Buffer.from('some secret', 'base64')),
secretFnBufferAsync: async (_req, _token) => Buffer.from('some secret', 'base64'),
secretFnCallback: (_req: any, _token: any, cb: any) => { cb(null, 'supersecret') },
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't the function arguments should infer the correct types here?
Providing :any seems to be breaking changes.

We may type the const as Record<string, FastifyJWTOptions['secret']

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have again issues with determining the right function type

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Aug 10, 2023

@climba03003

I somehow think that all plugins which use namespaces or custom defined values should have something like this helper interface.

I am currently thinking, if instead of having four generics, to have one generic and use that for the interface. Interface would be then identical to the option.

@Uzlopak Uzlopak merged commit 5ba0403 into master Aug 24, 2023
17 checks passed
@Uzlopak Uzlopak deleted the improve-types branch August 24, 2023 10:09
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.

None yet

3 participants