koajs / bodyparser

Koa body parsing middleware
MIT License
1.31k stars 117 forks source link

[fix] `encoding` is still required in TS even though it has default value from `co-body` #155

Closed JeromeDeLeon closed 6 months ago

JeromeDeLeon commented 1 year ago

Describe the bug

Node.js version: 18.6.1 OS version: macOS 12.6

Description: When I used the v5 of this lib, I am getting a type error from the encoding field being required but the README.md says that it has a default value suggesting it isn't required. https://github.com/koajs/bodyparser/blob/664cd7c413250d5e12eb5bb0fbf4e52d31ef24f5/README.md?plain=1#L50

Another thing is that encoding is not being passed on to the co-body's parse function so it seems useless compared to v4 where it uses copy-to's copy function to merge options. https://github.com/koajs/bodyparser/blob/5678a79e64d7833e0dd7734c9e9de40126e14d98/index.js#L50-L111

compared to v5 where it limits the options https://github.com/koajs/bodyparser/blob/664cd7c413250d5e12eb5bb0fbf4e52d31ef24f5/src/body-parser.ts#L63-L73

so I'm not entirely sure if we should document this as breaking changes that we're not able to override the encoding or any other values or fix this type of issue.

Also, I think it's an issue with co-body type declaration where they explicitly put undefined as part of the type telling us to explicitly pass a value of undefined

https://github.com/DefinitelyTyped/DefinitelyTyped/blob/def5996adb3596c8c468922ed0ac55110cd47fcc/types/co-body/index.d.ts#L33-L42

Actual behavior

Argument of type '{ jsonLimit: string; }' is not assignable to parameter of type 'BodyParserOptions'.
  Property 'encoding' is missing in type '{ jsonLimit: string; }' but required in type 'Pick<Options, "encoding">'.ts(2345)

Expected behavior

Should not be getting any type of error

Code to reproduce

For whatever reason, I wasn't able to replicate it in the TS playground but I was able to replicate it in CodeSandbox.

Workaround

bodyParser({ ...yourOptions, encoding: undefined }) // only if you're not overriding it

Checklist

3imed-jaberi commented 6 months ago

Thanks @JeromeDeLeon for your catch! Yup, it looks like an issue with the co-body, and from our side we don't pass the encoding option ... I am working on it and expect the new release soon!