apollo-server-integrations / apollo-server-integration-koa

An integration that lets you run your Apollo GraphQL server as part of an app built with Koa
MIT License
18 stars 4 forks source link

koa-bodyparser seems dead now; @koa/bodyparser is the up-to-date version #214

Open aseemk opened 6 months ago

aseemk commented 6 months ago

Hi there. Thanks for this library! We've been using it and it's helpful.

I recently ran into an issue unrelated to Apollo, but related to the Koa body-parsing middleware this library expects and requires.

We installed koa-bodyparser which this library's readme recommends. However, you can see that package is stuck on version 4.

If you go to the Koa bodyparser's GitHub repo, it actually specifies the package name as @koa/bodyparser — and that package is on version 5 now (and actually maintained).

I had to use a new option added to version 5 to fix a bug, and couldn't understand why there was no version 5 on npm.

Please consider upgrading not just the readme here but also the code and test dependencies on this package.

Thanks again and hope this helps others!

aseemk commented 6 months ago

Another issue related to this: going from v4 (koa-bodyparser) to v5, @koa/bodyparser now no longer populates ctx.request.body (even to {}) on GET requests:

https://github.com/koajs/bodyparser/blob/664cd7c413250d5e12eb5bb0fbf4e52d31ef24f5/src/body-parser.ts#L30

https://github.com/koajs/bodyparser/blob/664cd7c413250d5e12eb5bb0fbf4e52d31ef24f5/src/body-parser.ts#L76-L87

Makes sense since GET requests have no body, so should need no parsing.

But this Apollo Koa integration still indiscriminately expects a ctx.request.body to be set even if the method is GET (for loading the Apollo GraphiQL UI):

https://github.com/apollo-server-integrations/apollo-server-integration-koa/blob/71b7ba3d92ea5dbc53f73ddff52a11151dc393a6/src/index.ts#L72-L82

I'm hackily setting ctx.request.body ??= {} myself in my own wrapper middleware if ctx.method === 'GET', and this works. But should this be fixed here in this library? Thank you!

matthew-gordon commented 6 months ago

Hi @aseemk, please refer to this previous comment: https://github.com/apollo-server-integrations/apollo-server-integration-koa/issues/60#issuecomment-1335560399

Ultimately I agree It is certainly a good idea to use packages that are actively maintained, if you feel strongly I definitely encourage a PR with the changes.