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

When using typescript with @fastify/auth, verifyBearerAuth throws a "Type 'verifyBearerAuth | undefined' is not assignable to type 'FastifyAuthFunction | FastifyAuthFunction[]'" #176

Closed
2 tasks done
brunoshine opened this issue Jan 15, 2024 · 8 comments
Labels
good first issue Good for newcomers

Comments

@brunoshine
Copy link

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure the bug has not already been reported

Fastify version

4.25.2

Plugin version

9.3.0

Node.js version

20.9.0

Operating system

Linux

Operating system version (i.e. 20.04, 11.3, 10)

Ubuntu 22.04.2 LTS

Description

Picking up on the sample provided on the REAME for the integration with @fastify/auth, if we create a route with just the fastify.verifyBearerAuth we get a typescript error Type 'verifyBearerAuth | undefined' is not assignable to type 'FastifyAuthFunction | FastifyAuthFunction[]'. Type 'undefined' is not assignable to type 'FastifyAuthFunction | FastifyAuthFunction[]'.ts(2322)

  fastify.route({
      method: 'GET',
      url: '/bearer',
      preHandler: fastify.auth([fastify.verifyBearerAuth]),
      handler: function (_, reply) {
          reply.send({ hello: 'world' })
      }
  })

The workaround was to add a // @ts-ignore

 fastify.route({
        method: 'GET',
        url: '/bearer',
        // @ts-ignore <<<<<<<<<<<<<<< workaround
        preHandler: fastify.auth([fastify.verifyBearerAuth]),
        handler: function (_, reply) {
            reply.send({ hello: 'world' })
        }
    })

Any thoughts? thanks

Steps to Reproduce

Follow the README sample and add a route with just the verifybearerauth call:

    fastify.route({
        method: 'GET',
        url: '/bearer',
        preHandler: fastify.auth([fastify.verifyBearerAuth]),
        handler: function (_, reply) {
            reply.send({ hello: 'world' })
        }
    })

Expected Behavior

Should work without the error.

@mcollina
Copy link
Member

Thanks for reporting! Would you like to send a Pull Request to address this issue? Remember to add unit tests (we use tsd for typescript)

@mcollina mcollina added the good first issue Good for newcomers label Jan 15, 2024
@casantosmu
Copy link

verifyBearerAuth is declared as undefined here. This reflects the reality that Fastify cannot automatically determine the presence of specific decorators in each context.

To address this, you can manually check for verifyBearerAuth before defining the route:

if (!fastify.verifyBearerAuth) {
  throw new Error("Fastify verifyBearerAuth decorator required");
}

fastify.route({
  method: "GET",
  url: "/bearer",
  preHandler: fastify.auth([fastify.verifyBearerAuth]),
  handler: function (_, reply) {
    reply.send({ hello: "world" });
  },
});

However, many plugins in the Fastify ecosystem declare these decorators as defined. I could update the module declaration to remove the possibility of verifyBearerAuth being undefined, changing this:

declare module 'fastify' {
  interface FastifyInstance {
    verifyBearerAuthFactory?: fastifyBearerAuth.verifyBearerAuthFactory
    verifyBearerAuth?: fastifyBearerAuth.verifyBearerAuth
  }
}

To this:

declare module 'fastify' {
  interface FastifyInstance {
    verifyBearerAuthFactory: fastifyBearerAuth.verifyBearerAuthFactory
    verifyBearerAuth: fastifyBearerAuth.verifyBearerAuth
  }
}

@mcollina what do you think?

@mcollina
Copy link
Member

I would do that.

@climba03003
Copy link
Member

climba03003 commented Jan 15, 2024

It is marked as undefined because those methods is not added by default.
I believe extra checking on the existence is a good idea.

If it removes the ? part, I believe there would be a bug report asking why those methods is undefined.

@casantosmu
Copy link

I believe it is preferable to catch errors during the build process rather than encountering them at runtime.

@climba03003
Copy link
Member

climba03003 commented Jan 15, 2024

If you compare with other plugins, they exist in non-nullable methods only because they are always decorated.

I don't think the current types is wrong, change it is giving a false image to the users it is always there.

I don't want to be chasing it back and forth.

@casantosmu
Copy link

It seems there might be a confusion. I agree with you.

@Fdawgs
Copy link
Member

Fdawgs commented Dec 21, 2024

I think @casantosmu's solution is best for this, closing.

@Fdawgs Fdawgs closed this as completed Dec 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

5 participants