Aquila169 / zod-express-middleware

Express middleware to validate requests using zod schema's.
MIT License
86 stars 13 forks source link

Working with other middleware, spefically locals #6

Open nasainsbury opened 2 years ago

nasainsbury commented 2 years ago

Hi, thanks for this package, it's really useful.

I currently have the following example endpoints:

router.post("/", middleware.validator(schema.create), middleware.auth(), async (req, res) => {});

and 

router.post("/", middleware.validator(schema.create), async (req, res) => {});

Basically, I have created my own auth middleware that when it's used I want there to be a value in locals, say user, and when I'm not using the middleware, it's not present.

So far I have:

export const auth: <P, ResBody, ReqBody, ReqQuery, Locals>() => RequestHandler<
    P,
    ResBody,
    ReqBody,
    ReqQuery,
    Locals & { user: string }
> = () => (req, res, next) => {
    res.locals.user = "Nathan";
    return next();
};

I have removed functionality, but basically, this will mean i can access req.locals.user in my endpoint if using this middleware, and not if I'm not.

This works, however, when being used in conjunction with your middleware, it does not. I'm no TS expert, but I think your middleware may be overwriting res.locals.

I'm not sure where to go from here, and I'm not sure it's an issue with this library, but not sure on the best course of action.

timjam commented 1 year ago

I am having similar issue. If having a custom auth middleware injecting to res.locals the validateRequest somehow loses all the types from req and res. However, if the auth middleware is moved away from the router itself and into the server/app middlewares, then the problem doesn't exist.


const app = express()
app.use(customeAuthMW)
app.use('/someRoute', someRouter)

Now there, if the someRouter is only using the validateRequest it works. However, if doing this

const someRouter = Router()

someRouter.get('/typesDoesNotWorkHere', [customAuthMW], validateRequest({ .... }), async (req, res) => {
  // Type information is lost here
})

someRouter.get('/typesWorkHere', validateRequest({ ... }), async (req, res) => {
  // Types work here as intended
})
timjam commented 1 year ago

My original auth middleware was roughly like this

const authMW = async (req: express.Request, res: express:Response, next: NextFunction) => {
  try {
    // Do things
    const authInfo = await callToExternalAuthService()
    if (authInfo) {
       res.locals.authInfo = authInfo
    }
    next()
  } catch (error) {
    next(error)
  }
}

The async keyword of course forces the return type to be Promise<void>.

If I change the auth middleware to use then syntax

const authMW = (req: express.Request, res: express:Response, next: NextFunction) => {
    // Do things
    callToExternalAuthService()
      .then(authInfo => {
        if (authInfo) {
           res.locals.authInfo = authInfo
        }
        next()
       })
      .catch(error => next(error))
}

the return type is void and I can use the authMW

someRouter('/qwerty', authMW, validateRequest({ ... }), (req, res) => {
  // Types will be correct here
})

However, something interesting happens, if I pass the middlewares as an array.

someRouter('/qwerty', [authMW], validateRequest({ ... }), (req, res) => {
  // Types will be lost here
})
someRouter('/qwerty', [authMW, validateRequest({ ... })], (req, res) => {
  // With this I get huge TS error pointing to the validateRequest function
})
someRouter('/qwerty', [authMW, () => validateRequest({ ... })], (req, res) => {
  // TS will be happy about this, but the types are lost again
})

So there must be something I don't understand in how Express handles the middlewares and how the validateRequest works.