fastify-auth icon indicating copy to clipboard operation
fastify-auth copied to clipboard

Extend FastifyAuthFunction type to accept async functions

Open FerrielMelarpis opened this issue 9 months ago • 3 comments

Prerequisites

  • [x] I have written a descriptive issue title
  • [x] I have searched existing issues to ensure the feature has not already been requested

🚀 Feature Proposal

Context

The FastifyAuthFunction type provided only covers the callback-style function. This library supports both callback-style and promise-based functions.

[!NOTE] Not sure if this should be filed as bug. The feature is implemented for promise-based functions, it's just missing the types I need. Feel free to move this to appropriate place. 🙂

Proposal

Extend the FastifyAuthFunction type to be a union of the callback-style functions and promise-based functions.

Something like:

import type { FastifyInstance, FastifyRequest, FastifyReply, HookHandlerDoneFunction } from "fastify";

...

export FastifyAuthFunction =
  | ((this: FastifyInstance, request: FastifyRequest, reply: FastifyReply, done: HookHandlerDoneFunction) => void)
  | ((this: FastifyInstance, request: FastifyRequest, reply: FastifyReply) => Promise<void>);

Motivation

When creating several authentication strategies, I want their functions to have consistent signature. I started using the promise-based APIs in fastify and when I used this library, I found that typescript complains about a 4th argument when I have some tests calling the strategy function like this:

await strategyFn.call(mockFastifyInstance, mockRequest, mockReply); // Expects a 4th argument

This matches the current type definition wherein we should provide a done handler. However, that's not required for async functions.

Example

await strategyFn.call(mockFastifyInstance, mockRequest, mockReply); // valid in TS

FerrielMelarpis avatar Apr 30 '25 10:04 FerrielMelarpis

Thanks for the issue! Would you like to send a PR?

gurgunday avatar May 01 '25 05:05 gurgunday

I would like to open a PR for this issue due to contribute if you allow me. https://github.com/fastify/fastify-auth/pull/260

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 meet this requirement, also added async and promise-based tests to cover the update.

ozers avatar May 14 '25 22:05 ozers

Thanks @ozers I think I came up with a similar solution. Looking forward for your PR to get merged 👍🏼

FerrielMelarpis avatar May 15 '25 19:05 FerrielMelarpis