elierotenberg / fastify-zod

Zod integration with Fastify
MIT License
212 stars 19 forks source link

Route methods do not consider `prefix` in parent instance #23

Open revosw opened 2 years ago

revosw commented 2 years ago

In a setup I have, I make a subroute under /admin/:id. The normal instance.get() method in the subroute recognizes that the prefix has been added, so /ventilator is appended to /admin/:id. However, instance.zod.get() does not recognize the prefix, and insists that I have to assign /admin/:id/ventilator as the path.

export const runServer = async () => {
  const fastify = Fastify()

  // fastify-zod integration
  await register(fastify, {
    jsonSchemas: buildJsonSchemas(schemas),
    swaggerOptions: {
      routePrefix: '/swagger',
      exposeRoute: true,
    }
  })

  fastify.register(adminRoutes, {
    prefix: '/admin/:id'
  })

  const port = 3000
  fastify.listen({ port }, err => {
    if (err) throw err
    console.log(`Listening on port ${port}`)
  })
}

runServer()
import { FastifyPluginAsync } from 'fastify'
import { ObjectId } from 'mongodb'
import { Ventilator, ventilator } from '../../schemas/ventilator/ventilator'

const adminRoutes: FastifyPluginAsync = async (instance) => {
    // {"message":"Route GET:/admin/62765ac9c76bdc1796d81b89/ventilator not found","error":"Not Found","statusCode":404}
    instance.zod.get('/ventilator', {operationId: 'getVentilator', params: 'GetVentilatorHandlerParams', reply: 'ventilatorWithId'}, async ({params}) => {
        const document = await instance.mongo.db?.collection<Ventilator>('ventilators').findOne({vid: new ObjectId(params.vid)})
        if (document == null) throw new Error('Could not retrieve document.')
        return document
    })
    // {"vid":"62765ac9c76bdc1796d81b89"}
    instance.get<{Params: {vid: ObjectId, ventilatorid: ObjectId, itemid: ObjectId}}>('/ventilator', (req, res) => {
        res.send(req.params)
    })
}

export default adminRoutes
elierotenberg commented 2 years ago

Thank you for pointing that out. Would you please create a failing test case I can use to implement a fix? Otherwise I'll do it.

revosw commented 2 years ago

I'm sorry but I didn't commit the code before migrating to typebox, so the state of the project when I made that code is gone. I sadly don't have the energy to set up a minimal repro

r0mankon commented 1 year ago

Is this going anywhere? Also the case for fastify-autoload plugin: #34

arnorhs commented 1 year ago

this makes the fastify.zod[get|post|etc] is completely incompatible with my setup and i'll have to continue the "deprecated" route..

elierotenberg commented 1 year ago

I am very interested in supporting this. Can you please provide me with some example code?

arnorhs commented 1 year ago

@elierotenberg so for context, a lot of people structure their routes like this with fastify:

app.register((i, r, d) => {
  i.register(hotTakesRoutes, {
    prefix: 'hot-takes'
  })

  done()
}, {
  prefix: 'api',
})

this is super handy since you can inject different stuff into the request depending on the request group - they all share the prefix, you can add extra guards etc

it's explained in more detail here: https://www.fastify.io/docs/latest/Reference/Plugins/

moreover a lot of people also prefer the instance.route() way of defining routes.. ie.

instance.route<{ Reply: SomeDef }>({
  method: 'GET',
  url: '/someurl',
  preHandler: (req) => { /* check something, or query for data */ },
  handler: async () => {
    ...route logic
  }
})

I'm not here to argue for why somebody would do this, this is just part of the fastify api. and the reason why the very nice and clever looking api instance.zod.xxx() won't work for a lot of fastify use cases.

I hope that clarifies the issue and is helpful

balkarika commented 1 year ago

Is there a way I can help with this issue? Sample github project or something?

atila29 commented 7 months ago

Is there any news on this?