search

bithavoc / express-winston

express.js middleware for winstonjs
https://www.npmjs.com/package/express-winston
MIT License
796 stars 187 forks source link

Extend Request type with responseTime and body #233

Closed draperunner closed 2 years ago

draperunner commented 4 years ago

The logger middleware alters the response object, so that its type is not exactly like the pure Express request. This leads to errors if you're using res.responseTime or res.body in your own dynamicMeta. I have created a type ExpressWinstonResponse which includes these two fields.

I have also made the DynamicMetaFunction use this new ExpressWinstonResponse type. But I had to split it, as the error middleware's response does not have a responseTime or body. So the type for dynamicMeta is different for the error middleware and the standard middleware.

I have also replace object with Record<string, any> as the return type for dynamicMeta. This is because if object is used, TypeScript does not allow you to add properties to the returned object.

bithavoc commented 4 years ago

Question, is this a breaking change for TS? can I interchangeably use a previous method with signature DynamicMetaFunction in the new property of base logger of type DynamicMetaFunctionError?

yinzara commented 4 years ago

I think if you just made the request and response fields optional in the interface, you wouldn't need a separate DynamicMetaFunctionError and DynamicLevelFunctionError (i.e. you could use the same for the error handler as the regular request handler.

And yes @bithavoc I think his changes would be TS compatible (i.e. they would not be a breaking change) but I think it's worth having tests that prove that.