pino-http
pino-http copied to clipboard
pinojs
Pino 7 TypeScript
Now pino 7 is including TypeScript types, would it make sense to include TypeScript types in this package as well? The types in DefinitelyTyped for this package don't work with the pino 7 types.
Would you like to send a Pull Request to address this issue? Remember to add unit tests.
cc @kibertoad
I don't have any experience adding types to a project. Would it be acceptable to copy the types from definitely typed and fix the errors?
Hello 👋 are there any updates on this issue? Are types going to be included in pino-http? I wouldn't mind trying and start a pr with your help
A PR would be nice.
Hi @mcollina, I opened a PR with the type definition. let me know what do you think. 🚀
Am I missing some required changes after the typings now being included in the package itself?
The IncomingMessage declaration for a 'req' property is missing in types now, which means TS will complain about doing `req.log.info()' in Express handlers/middleware.
This module declaration was included in the typings from @types/pino-http, but does not exist in the typings included in pino-http itself now: (from https://github.com/DefinitelyTyped/DefinitelyTyped/blob/5bdfa971b6a8422f8fe1d7ea2a87f1f1e911a8fe/types/pino-http/index.d.ts#L66)
declare module 'http' {
interface IncomingMessage {
id: PinoHttp.ReqId;
log: Logger;
}
interface ServerResponse {
err?: Error | undefined;
}
interface OutgoingMessage {
[PinoHttp.startTime]: number;
}
}
https://github.com/pinojs/pino-http/pull/184 I think this one "removed" some PinoHttp.Options from types. They aren't being merged properly. I am using NestJS Pino Logger and using the redact option.
LoggerModule.forRoot({
pinoHttp: [
{
redact: [
'req.headers.authorization',
'req.headers.cookie',
'req.headers["x-csrf-token"]'
]
}
]
}),
It was yelling at me once I upgraded to the latest version. Moving back to 6.3.0 fixed it. I haven't figured out how that PR could have broken the types. I need to take a look again and see if I can make a PR.
#184 I think this one "removed" some PinoHttp.Options from types. They aren't being merged properly. I am using NestJS Pino Logger and using the
redactoption.LoggerModule.forRoot({ pinoHttp: [ { redact: [ 'req.headers.authorization', 'req.headers.cookie', 'req.headers["x-csrf-token"]' ] } ] }),It was yelling at me once I upgraded to the latest version. Moving back to
6.3.0fixed it. I haven't figured out how that PR could have broken the types. I need to take a look again and see if I can make a PR.
Yeah. I'm working on a fix for this in nestjs-pino, but first I need #188 merge into master
