fastify / fastify-type-provider-typebox

A Type Provider for Typebox
MIT License
153 stars 26 forks source link

preHandler types conflict with package `@fastify/auth` #5

Closed TheWashiba closed 2 years ago

TheWashiba commented 2 years ago

Prerequisites

Fastify version

4.0.0-rc.3

Plugin version

No response

Node.js version

16.x

Operating system

Linux

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

Ubuntu 20.04

Description

I'm having TypeScript issues in the route preHandler when the package @fastify/auth ^3.0.0 is being used altogether. I'm unsure if this an issue of that package or this, but I've began experiencing issues once .withTypeProvider<TypeBoxTypeProvider>() is called. The error received:

Type 'preHandlerHookHandler<Server, IncomingMessage, ServerResponse, RouteGenericInterface, unknown, FastifySchema, FastifyTypeProviderDefault, ResolveFastifyRequestType<...>, FastifyLoggerInstance>' is not assignable to type 'preHandlerHookHandler<Server, IncomingMessage, ServerResponse, RouteGenericInterface, unknown, FastifySchema, TypeBoxTypeProvider, ResolveFastifyRequestType<...>, FastifyLoggerInstance> | preHandlerHookHandler<...>[] | undefined'.

Steps to Reproduce

Install fastify@4, @fastify/auth@3 and .1.0-beta.1 version of this plugin in a TypeScript environment. Calling the fastify.auth([]) in the route preHandler will give a type error.

Expected Behavior

No response

mcollina commented 2 years ago

cc @fastify/typescript could you take a look?

mcollina commented 2 years ago

Could you create a repo with the setup that is not working?

TheWashiba commented 2 years ago

@mcollina here you go https://github.com/TheWashiba/fastify-typescript-bug

mcollina commented 2 years ago

thx, I'll take a look asap

mcollina commented 2 years ago

@fastify/typescript can you take a look as well? I'm kind of unsure what's the problem.

mcollina commented 2 years ago

@kibertoad @fox1t @Eomm could you take a look?

kibertoad commented 2 years ago

on it

TheWashiba commented 2 years ago

Any news about it?

fox1t commented 2 years ago

First of all, sorry for the late response. I've just looked into this, and it turns out that the issue is indeed the .withTypeProvider<TypeBoxTypeProvider>() call.

`@fastify/auth arguments fastify instance with auth method that is a preHandlerHookHandler that uses the default type provider https://github.com/fastify/fastify-auth/blob/master/auth.d.ts#L18 This clashes with your fastify instance that uses TypeBox TypeProvider.

Unfortunately, at the moment, I don't have a solution. I will look more deeply into this and return if anything comes to mind.

mcollina commented 2 years ago

cc @sinclairzx81

sinclairzx81 commented 2 years ago

@mcollina @TheWashiba Hi. Had a brief look at this the other day. The issue appears to be caused by the server.auth([]) function returning a function signature that is globally typed to use the FastifyTypeProvider (not the TypeBoxTypeProvider as configured). As such the assignment of server.auth([]) on the preHandler is the primary source of this type error as the Provider types are incompatible (as mentioned by @fox1t )

Unfortunately, because the auth() function is globally typed (using module augmentation here), and because the plugin is patching the auth() function at runtime on the fastify instance, there doesn't appear to be many options available to resolve the local typing at the provider, as TypeScript is unaware of any type modifications made by the patch.

Workaround

In the interim, the following should offer a workaround if using Type Providers (albeit a not ideal solution with the any). @TheWashiba this code was derived from your repro repository. You can preview the typing on this located here.

import { Type } from '@sinclair/typebox'
import { TypeBoxTypeProvider } from '@fastify/type-provider-typebox'
import fastifyAuth from '@fastify/auth'
import fastify from 'fastify'
import fp from 'fastify-plugin'

// --------------------------------------------------------------------------------
// Plugin
// --------------------------------------------------------------------------------

export const plugin = fp(
  (fastify) => {
    const instance = fastify.withTypeProvider<TypeBoxTypeProvider>();

    instance.route({
      method: 'GET',
      url: '/plugin',
      schema: {
        body: Type.Object({
          a: Type.Number(),
          b: Type.Number()
        })
      },
      preHandler: instance.auth([]) as any, // todo: review global module augmentation of the server auth() function.
      handler: async (req, res) => { 
        const { a, b } = req.body 
      },
    });
  },
  {
    name: 'example',
  }
);

// --------------------------------------------------------------------------------
// Index
// --------------------------------------------------------------------------------

const server = fastify().withTypeProvider<TypeBoxTypeProvider>();
server.register(fastifyAuth);
server.register(plugin)
server.route({
  method: 'GET',
  url: '/',
  schema: {
    body: Type.Object({
      x: Type.Number(),
      y: Type.Number()
    })
  },
  preHandler: server.auth([]) as any, // todo: review global module augmentation of the server auth() function.
  handler: async (req, res) => {
    const { x, y } = req.body
  },
});

Possible Next Steps

I think probably the best way forward is spending some time taking a look at the way the current server.auth([]) function definitions (potentially avoiding global module augmentation if possible). I think any investigations into this would likely be applicable for other plugin modules that augment the underlying FastifyInstance. So would make for good documentation for plugin authors writing TS definitions.

In terms of TypeScript, the ideal way to allow types to propagate would be to chain calls to register(), as follows.

const server = fastify()
 .withTypeProvider<TypeBoxTypeProvider>() // remap to generic TypeBoxTypeProvider
 .register(fastifyAuth)                   // augment instance with `.auth()` function.
 .register(plugin)                        // augment with user defined functionality.
 .route({                                 // route: configured with the above
   method: 'GET',
   url: '/',
   schema: {
     body: Type.Object({
       x: Type.Number(),
       y: Type.Number()
     })
   },
   preHandler: server.auth([]),
   handler: a sync (req, res) => {
     const {  x, y } = req.body
   }
  });

Although there may be additional options to explore with the following PR's

https://github.com/fastify/fastify/pull/3952 - Global augmentation of FastifyTypeProviderDefault https://github.com/fastify/fastify/pull/4034 - Propagate generics from FastifyRegister

Hope that helps! S

mcollina commented 2 years ago

@fox1t @climba03003 could any of you take ownership of this? it looks like it's the final step for our TS definitions.

climba03003 commented 2 years ago

The next steps solution cannot be implemented without a breaking change. As a quick but not a good fix, we can loosen the type on fastify-auth to preHandlerHookHandler<any, any, any, any, any, any, any, any, any> or do the similar thing on fastify-websocket.

declare module 'fastify' {
  interface FastifyInstance<RawServer, RawRequest, RawReply, Logger, TypeProvider> {
    auth(
      functions: FastifyAuthFunction[],
      options?: {
        relation?: 'and' | 'or',
        run?: 'all'
      }
    ): preHandlerHookHandler<RawServer, RawRequest, RawReply, RouteGenericInterface, ContextConfigDefault, FastifySchema, TypeProvider>;
  }
}
mcollina commented 2 years ago

The next steps solution cannot be implemented without a breaking change.

Maybe open a tracking issue on Fastify? It could be the basis of Fastify v5.


Regarding the fix on fastify-auth, go for it. It seems the generic one is better, but that's my 2 cents.

mcollina commented 2 years ago

The change in fastify landed. I would like to close this and move the discussion about the follow-up in Fastify proper. Could you open such an issue @climba03003?

irg1008 commented 1 year ago

Hi, I have searched and searched but It seems there is no clear solution to this specific problem yet.

Do you know a way to avoid having to put "as any" everywhere?

nahtnam commented 1 year ago

Hi, just wanted to add onto this ticket here, I'm also unable to get it working without using as any. Here are the relevant snippets of code. Am I doing something wrong?

export const fastify = FastifyServer({}).withTypeProvider<ZodTypeProvider>();

fastify.decorate('verifyServerSecret', async (request: FastifyRequest) => {
  if (request.headers.authorization !== `Bearer ${env.SERVER_SECRET}`) {
    throw new UNAUTHORIZED("You don't have access to this endpoint");
  }
});

fastify.register(fastifyAuth);

declare module 'fastify' {
  interface FastifyInstance {
    verifyServerSecret: FastifyAuthFunction;  // technically this typing is wrong because I use `async/await` but it still works
  }
}

export type FastifyInstance = typeof fastify;

And then used like so:

fastify.route({
    method: 'POST',
    url: '/test',
    schema: {
      body: z.object({
        record: z.object({
          id: z.string(),
        }),
      }),
      response: {
        200: z.object({}),
      },
    },
    preHandler: fastify.auth([fastify.verifyServerSecret]), // adding `as any` here fixes it but doesn't seem right
    async handler(req) {
      const {
        record: { id },
      } = req.body; // ERROR: typed incorrectly :(

      return {};
    },
  });
nahtnam commented 1 year ago

Ideally I want to get more complicated and do something like this:

fastify.decorate('authenticateUser', async (request: FastifyRequest) => {
  // check headers and fetch user
  req.user = {} as User
});

where req.user is correctly typed:

fastify.route({
    // ...
    preHandler: fastify.auth([fastify.authenticateUser]),
    async handler(req) {
      // req.user works and is correctly typed but only on routes that define the `preHandler`
    },
  });

it this a possibility?

ekoeryanto commented 7 months ago

Hi, to prevent my self confused again. the solution is only to cast to never or any the prehandler as it impact nothing

fastify.route({
    // ...
    preHandler: [] as never,
    // ...
  });
JonRC commented 4 months ago

Guys, do that and your types will work as expected.

export const yourPreHandler = async <
  Request extends FastifyRequest,
  Reply extends FastifyReply
>(
  request: Request,
  reply: Reply
) => {
  // Your code here
};