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

Typescript: Type Provider information gets lost with onRequest #4120

Closed
2 tasks done
qqilihq opened this issue Jul 8, 2022 · 11 comments
Closed
2 tasks done

Typescript: Type Provider information gets lost with onRequest #4120

qqilihq opened this issue Jul 8, 2022 · 11 comments
Labels
typescript TypeScript related

Comments

@qqilihq
Copy link

qqilihq commented Jul 8, 2022

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.2.0

Plugin version

No response

Node.js version

16.13.0

Operating system

macOS

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

n/a

Description

I've added the new type providers from Fastify 4 to my project to avoid the redundant, cluttered generics.

My project makes use of “onRequest” hooks which check a custom authentication in different endpoints.

Example:

// simplified for demo purposes -- this is used in various endpoints
const authenticateHook: onRequestAsyncHookHandler<any, any, any, any, any, any, any, any, any> = () => {
  return Promise.resolve();
};

// example copied from https://www.fastify.io/docs/latest/Reference/Type-Providers/#typebox
server.get('/route', {
    schema: {
        querystring: Type.Object({
            foo: Type.Number(),
            bar: Type.String()
        })
    },
    onRequest: authenticateHook
}, (request, reply) => {
    // type Query = { foo: number, bar: string }
    const { foo, bar } = request.query // NOT type safe when adding `onRequest` :-(
})

The problem: After adding the onRequest hook, the type safety gets totally lost and e.g. request.query is unknown.

I've been trying various workarounds (e.g. handling through all these generics), however I didn't really get to a clean, let alone, scalable solution.

Steps to Reproduce

See snippet.

Expected Behavior

I'd expect the type information to be preserved.

@mcollina
Copy link
Member

mcollina commented Jul 8, 2022

@fastify/typescript could you take a look?

@sinclairzx81
Copy link
Contributor

@qqilihq Hi! Does the following help? TypeScript Link Here

async function authenticateHook<T extends FastifyRequest, U extends FastifyReply>(request: T, reply: U) {
  // note: shared hooks may have varying generic params per route, so request.query should be unknown
}

Fastify().withTypeProvider<TypeBoxTypeProvider>().get('/route', {
    onRequest: (request, reply) => authenticateHook(request, reply), // no bind
    schema: {
        querystring: Type.Object({
            foo: Type.Number(),
            bar: Type.String()
        })
    }
}, (request, reply) => {
    const { foo, bar } = request.query // ok
})

@qqilihq
Copy link
Author

qqilihq commented Jul 8, 2022

Hi @sinclairzx81 -- great, that indeed does the trick. Thank you very much!

@mcollina
Copy link
Member

mcollina commented Jul 8, 2022

I think we should document that, would you like to send a PR?

@RafaelGSS
Copy link
Member

Isn't it a bug? @sinclairzx81 It seems that the type should be inferred even in a bind. I'm missing something?

@sinclairzx81
Copy link
Contributor

sinclairzx81 commented Jul 9, 2022

@RafaelGSS Hi! Yeah, it's a bug. Had a quick look into it last night, but did not find the cause (so suggested the workaround in the interim). I may do a bit more investigation into this over the next few days.

@sinclairzx81
Copy link
Contributor

@RafaelGSS See PR for potential fix

@RafaelGSS
Copy link
Member

Thank you! I'm planning to look at it tomorrow

@climba03003 climba03003 added the typescript TypeScript related label Jul 21, 2022
@qqilihq
Copy link
Author

qqilihq commented Jul 21, 2022

Thank you all!

@ryanstaniforth
Copy link

@sinclairzx81 Are you still investigating the bind issue? I'm also having this problem. Thank you.

@sinclairzx81
Copy link
Contributor

@ryanstaniforth Hi. I'm not currently looking into this issue. But see examples below if they are helpful.

Shared Handlers

The following sets up a shared external handler. Note that because an external handler may be shared across multiple routes, we will want to treat any route parameters as unknown. The following achieves this.

TypeScript Link Here

// -----------------------------------------------------------------
// Fastify Request Shared
// -----------------------------------------------------------------

export type FastifyRequestShared = FastifyRequest<{}, any, any, any>

// -----------------------------------------------------------------
// Example
// -----------------------------------------------------------------

function SharedHandler(request: FastifyRequestShared, reply: FastifyReply, done: HookHandlerDoneFunction) {
  // const { x, y } = request.body  // shared: not allowed
}

Fastify().withTypeProvider<TypeBoxTypeProvider>().post('/',
  {
    onRequest: SharedHandler, 
    schema: {
      body: Type.Object({
        x: Type.Number(),
        y: Type.Number()
      }),
      response: {
        200: Type.String()
      }
    }
  },
  (req, res) => {
    const { x, y } = req.body

    res.send('hello')
  }
)

Inferred Handlers

In instances where you do want to infer the parameters for external handler, you will need to pull the schema of the route so it can be seen by the external handler. The following sets up the FastifyRequestInferred type to infer request parameters for the route, allowing the request.body to be inferred.

TypeScript Link Here

// -----------------------------------------------------------------
// Fastify Request Inferred
// -----------------------------------------------------------------

export type FastifyRequestInferred<
  Provider extends FastifyTypeProvider, 
  Schema extends FastifySchema
> = FastifyRequest<{}, any, any, Schema, Provider>

// -----------------------------------------------------------------
// External Schema observable to handler and route
// -----------------------------------------------------------------

const Schema = {
  body: Type.Object({
    x: Type.Number(),
    y: Type.Number()
  }),
  response: {
    200: Type.String()
  }
}

// -----------------------------------------------------------------
// Example
// -----------------------------------------------------------------

function InferredHandler(request: FastifyRequestInferred<TypeBoxTypeProvider, typeof Schema>, reply: FastifyReply, done: HookHandlerDoneFunction) {
  const { x, y } = request.body  // allowed
}

Fastify().withTypeProvider<TypeBoxTypeProvider>().post('/',
  {
    onRequest: InferredHandler,
    schema: Schema,
  },
  (req, res) => {
    const { x, y } = req.body

    res.send('hello')
  }
)

At this stage, there is still a requirement to setup the appropriate types to allow type provider generics to propagate for inference, so setting up custom generics can be necessary when using type providers in this way. I think there may be some upstream considerations going into making it a little bit easier to create types where needed (possibly provided as new utility types, or possibly a refactor for Fastify v5, see issue #4081).

Hope this helps!
S

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
typescript TypeScript related
Projects
None yet
Development

No branches or pull requests

6 participants