Add #94 Type support for http-logger added #178
Conversation
ThulinaWickramasinghe
changed the title
ThulinaWickramasinghe
changed the title
packages/http-logger/package.json
Outdated
| "main": "dist/index.js", | ||
| "scripts": { | ||
| "build": "node ../../scripts/esbuild.config.js", | ||
| "types": "types/index.d.ts", |
There was a problem hiding this comment.
This is in the wrong place, shall we bring it out ?
There was a problem hiding this comment.
Sorry it was mistakenly added. Fixed it.
There was a problem hiding this comment.
Added the requested changes.
| */ | ||
| type HttpLoggerOptions = { | ||
| whitelists?: string[]; | ||
| loggable?: string[] | ((req: Request) => Record<string, any>[]); |
There was a problem hiding this comment.
Since you've mentioned the entire express Request type here, shall we update the actual code to pass in the entire req as well, right now it's just passing the headers and body. Passing just the 2 of them seems pointless once i think of it again now. Also shall we add express to peer deps of this package.json file?
There was a problem hiding this comment.
Okay will do.
There was a problem hiding this comment.
Done
There was a problem hiding this comment.
@Akalanka47000 can you merge this.
| }, | ||
| "packageManager": "[email protected]" | ||
| "node": ">=14.0.0", | ||
| "pnpm": ">=7.5.0" |
There was a problem hiding this comment.
I searched a bit since I haven't seen pnpm being used here before, I found a few issues saying that this isn't supported, is it possible for you to see if it works right now, maybe try installing an older version of pnpm in a codespace and try pnpm i
There was a problem hiding this comment.
Will try.
There was a problem hiding this comment.
Seems to be working fine.
No description provided.