Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update to latest nestjs everywhere #56

Open
8 tasks
jsaur opened this issue Oct 21, 2021 · 3 comments
Open
8 tasks

Update to latest nestjs everywhere #56

jsaur opened this issue Oct 21, 2021 · 3 comments
Assignees
Milestone

Comments

@jsaur
Copy link
Contributor

jsaur commented Oct 21, 2021

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 jsaur self-assigned this Oct 21, 2021
@jsaur jsaur added this to the Kiva-21.22 milestone Oct 21, 2021
@jsaur jsaur assigned voutasaurus and unassigned jsaur Oct 25, 2021
@jsaur
Copy link
Contributor Author

jsaur commented Oct 25, 2021

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
Copy link
Collaborator

matt-raffel-kiva commented Oct 25, 2021

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
Copy link

ghost commented Oct 25, 2021

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.

This was referenced Nov 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants