kiva / protocol-common

Shared code across the various protocol microservices
Apache License 2.0
10 stars 3 forks source link

Update to latest nestjs everywhere #56

Open jsaur opened 2 years ago

jsaur commented 2 years ago

We've updated a major version of nestjs in protocol-common, so we'll need to update nestjs downstream to ensure type compatibility

EDIT: public repos that need updating:

jsaur commented 2 years ago

Here’s what I’m thinking may be the best approach: First in protocol-common we roll back the package.json version updates from https://github.com/kiva/protocol-common/pull/53/files and https://github.com/kiva/protocol-common/pull/55/files and then bump the patch version of protocol-common to 0.1.50. That way any other repo that uses protocol-common and runs npm install will be safe (ie no breaking changes). Then in a new PR we bump the major version of protocol-common to 1.0.0 and bring forward all those package updates from the 2 previous PRs (plus any other package updates we want). Then in each repo that uses protocol-common we can bump the major version manually, and then do the necessary code changes required to work with the new dependency versions (eg import the HttpModule from @nestjs/axios instead of @nestjs/common). FYI @voutasaurus @matt-raffel-kiva @jeffk-kiva

matt-raffel-kiva commented 2 years ago

Keep in mind, going to the new version of nestjs breaks some things in downstream dependencies. I remember some problems with Reflector injection in the LoggingInterceptor. Decisions will need to be made to figure out if the change belongs in protocol-common or elsewhere.

ghost commented 2 years ago

This makes a lot of sense. I should've made it a major version update to begin with. The change does unfortunately belong in protocol-common because like half of protocol-common uses these dependencies already. But yeah, maybe there are some things we can pull out and just implement on a per-service basis.