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

chore(deps): upgrade @nestjs to 9.3.9 #693

Merged
merged 23 commits into from
Mar 31, 2023
Merged

chore(deps): upgrade @nestjs to 9.3.9 #693

merged 23 commits into from
Mar 31, 2023

Conversation

bahrmichael
Copy link
Contributor

This PR upgrades the @nestjs libraries which were previously at 8.4.4 to 9.3.9.

@wing328
Copy link
Member

wing328 commented Mar 16, 2023

@bahrmichael thanks for the PR.

The build failed. Can you pleas take a look when you've time?

@bahrmichael
Copy link
Contributor Author

@wing328 I added a TS instruction, assuming TS became more strict over time and that the previous build failure is not a problem.

@wing328
Copy link
Member

wing328 commented Mar 24, 2023

I added a TS instruction, assuming TS became more strict over time and that the previous build failure is not a problem.

can we make it less strict for the time being so that the build passes?

what about updating the build to make the tests pass instead?

@bahrmichael
Copy link
Contributor Author

I'm not sure why the build fails sometimes, and I don't have permission to rerun it. It succeeds locally when I run it with node@16.19.1. Seeing the renovate changes where it sometimes failed and sometimes succeeds it feels like this is out of my control.

can we make it less strict for the time being so that the build passes?

I couldn't find any information how to do that. TS becoming more strict is just a guess, that might be related to other upgrades. Reverting them is not a good idea just to not have the ts-ignore line.

@OptoCloud
Copy link

Why not just cast it to any if you are going to ignore the error either way?

(error.response.data as any).on('data', data => this.logger.log(data.toString('utf8')));

@bahrmichael
Copy link
Contributor Author

Why not just cast it to any if you are going to ignore the error either way?

(error.response.data as any).on('data', data => this.logger.log(data.toString('utf8')));

Good idea! I added this change :)

@trunneml
Copy link

Could someone please approve the workflow. This PR upgrades a dep with a security issue.

@bahrmichael
Copy link
Contributor Author

@wing328 friendly bump :)

@wing328
Copy link
Member

wing328 commented Mar 31, 2023

approved the workflow to run. let's see how that goes.

@wing328
Copy link
Member

wing328 commented Mar 31, 2023

I noticed that the build passed via 5c81540

and the commit after that (6a0140c) broke the build.

is 6a0140c critical? can it be reverted for the time being to see if the build passes again?

@wiesnery
Copy link

@wing328 this is the second issue, that I see idle for a while and blocked me. Can we work on the community management, so that more members look over issues more frequently? This project is also used in business contexts but the speed of issue management almost holds me back from promoting it any further in sensitive environments.

@wiesnery
Copy link

For the time being: This dependably PR seems to build smoothly: #694

A lot of people are waiting on it.

@wing328
Copy link
Member

wing328 commented Mar 31, 2023

The best way to make certain issues a higher priority is via sponsorship: https://opencollective.com/openapi_generator

@bahrmichael
Copy link
Contributor Author

I added a revert, and things work fine locally. I'm going to unsubscribe from this issue and trust that you will resolve the security issue soon.

For anyone who doesn't want to wait for this PR, you can use overrides in the package.json.

  "overrides": {
    "@nestjs/common": "9.3.9",
    "@nestjs/core": "9.3.9",
    "@nestjs/testing": "9.3.9"
  }

@wing328
Copy link
Member

wing328 commented Mar 31, 2023

now all tests passed as indicated in ae627d1

thanks for the PR.

@wing328 wing328 merged commit 9e6f443 into OpenAPITools:master Mar 31, 2023
3 checks passed
@wing328
Copy link
Member

wing328 commented Mar 31, 2023

FYI. Merged #699 in order for the build/release to pass in the master.

@wing328
Copy link
Member

wing328 commented Apr 11, 2023

@bahrmichael thanks again for the PR 🙏

I triggered a release and when I tried to install the latest openapi-generator-cli with npm install -g @openapitools/openapi-generator-cli, I got the following warnings:

➜  openapi-generator7 git:(master) npm install -g @openapitools/openapi-generator-cli

npm WARN ERESOLVE overriding peer dependency
npm WARN While resolving: @nestjs/axios@0.0.8
npm WARN Found: @nestjs/common@9.3.11
npm WARN node_modules/@openapitools/openapi-generator-cli/node_modules/@nestjs/common
npm WARN   @nestjs/common@"9.3.11" from @openapitools/openapi-generator-cli@2.6.0
npm WARN   node_modules/@openapitools/openapi-generator-cli
npm WARN     @openapitools/openapi-generator-cli@"*" from the root project
npm WARN   1 more (@nestjs/core)
npm WARN 
npm WARN Could not resolve dependency:
npm WARN peer @nestjs/common@"^7.0.0 || ^8.0.0" from @nestjs/axios@0.0.8
npm WARN node_modules/@openapitools/openapi-generator-cli/node_modules/@nestjs/axios
npm WARN   @nestjs/axios@"0.0.8" from @openapitools/openapi-generator-cli@2.6.0
npm WARN   node_modules/@openapitools/openapi-generator-cli
npm WARN 
npm WARN Conflicting peer dependency: @nestjs/common@8.4.7
npm WARN node_modules/@nestjs/common
npm WARN   peer @nestjs/common@"^7.0.0 || ^8.0.0" from @nestjs/axios@0.0.8
npm WARN   node_modules/@openapitools/openapi-generator-cli/node_modules/@nestjs/axios
npm WARN     @nestjs/axios@"0.0.8" from @openapitools/openapi-generator-cli@2.6.0
npm WARN     node_modules/@openapitools/openapi-generator-cli

added 9 packages, removed 2 packages, and changed 102 packages in 43s

23 packages are looking for funding
  run `npm fund` for details

Is that something you can help us with by filing a PR to address these warnings?

@wing328
Copy link
Member

wing328 commented Apr 21, 2023

@wiesnery if you want to, you can now sponsor https://github.com/sponsors/kay-schecker (the author of openapi-generator-cli) directly to financially support his awesome open-source work.

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

Successfully merging this pull request may close these issues.

None yet

5 participants