search

expressjs / body-parser

Node.js body parsing middleware
MIT License
5.44k stars 727 forks source link

Update iconv-lite to latest 0.6.3 #476

Closed jperelli closed 3 months ago

jperelli commented 1 year ago

In a project I work we use body-parser and mysql2 as dependencies. We are running a very common combination of prisma+mysql2 / expressjs+bodyparser in vercel, and every bundle kb is important in cold start lambda functions.

mysql2 depends on iconv-lite: 0.6.3 and body-parser depends on iconv-lite: 0.4.24

I think it can be upgraded in body-parser to have the same library and avoid code duplication. This would save around 600kb (~500kb of duplication + ~100kb of improved bundle size in 0.6.0).

Thank you for this great project!

dougwilson commented 1 year ago

Hello, and sorry for the delay on responding; I didn't realize I never did. I took a look and it seems that there are some breaking changes in that upgrade path. I will, however, work to get it fully upgraded in our 2.0 branch 👍

RahulGautamSingh commented 1 year ago

Maybe using a automatic dependency bot like: renovate or dependabot can be useful? As the automated PRs will make it easier to see the errors/problems that might come with new updates and making the issues visible for contributors💪 I would be happy to set them up if that's okay.☺

dougwilson commented 1 year ago

Hi @RahulGautamSingh I'm not sure how that would help here, as it is not upgraded in 1.x because it has breaking changes that requires a major version bump for body-parser.

RahulGautamSingh commented 1 year ago

Regular update PRs will help keep all deps up-to-date, and if some dep-updates require version bump then those can be targeted to the branch which is under work ie. 2.0 branch.

dougwilson commented 1 year ago

Thanks, but I don't think having that is necessary; I already use a service that shows all ourdated on a central dashboard.

RahulGautamSingh commented 1 year ago

No problem! I thought I should comment once as you mentioned that you forgot to upgrade the dep and automated PRs will be handy there. ✌

Thanks for all the work you have put in making and maintaining project, it really help a lot!♥

dougwilson commented 1 year ago

Sorry, I meant I forgot to reply to this issue; not that I forgot to update the dep. I already knew it was out of date but incompatible.

Luzgan commented 3 months ago

2.0 is still ongoing right? I am asking because now I happened to need newer version of iconv, since there is a need to support ISO-8859-1, which is not included in older version :)

wesleytodd commented 3 months ago

Please open a PR against the 2.x branch (#66) if it is not already updated in there.