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

node and npm version issues #801

Closed
derberg opened this issue Jun 23, 2022 · 6 comments · Fixed by #815
Closed

node and npm version issues #801

derberg opened this issue Jun 23, 2022 · 6 comments · Fixed by #815
Labels
area/ci-cd Specify what technical area given issue relates to. Its goal is to ease filtering good first issues. area/javascript Specify what technical area given issue relates to. Its goal is to ease filtering good first issues.

Comments

@derberg
Copy link
Member

derberg commented Jun 23, 2022

Where do I even start...got for sure triggered by failing test in #799

We started having a problem with tests in this project. We run them at the moment on nodejs 12,14,15 and 15 is causing issues. This is easy, we can just switch to 16 and 15 is anyway not LTS.

When we switch to 16, which we should do anyway, we get an issue that semantic-release must be bumped because the version doesn't match the Node release. Easy, just bump the package

And then the problem starts, we get a bunch of issues related to peerDependency in release-related plugins as they are referring wrong versions, all that stuff.

Now wait, why these issues did not pop up before? it is because according to my knowledge early versions of node 16 had npm 7 while the latest versions include npm 8.

I tried to solve peer dependency issues - it was good. Worked like a charm...until I thought, I'll check with Node 12...and it failed as latest semantic-release do not support Node 12


There are 2 threads in this issue, 2 different challenges:

  • Node 12 is no longer maintained by Node, and IMHO we should just drop it....but this is bigger, we have same release and pr testing workflows everywhere, we need to agree on dropping Node 12 everywhere, and the ones that do not agree, will need to maintain their custom workflows 🤷🏼
  • I think I made a fundamental mistake in 2020 when I designed the release pipeline by managing the dependencies through the devDependencies in package.json. In my opinion to make it easier in the future, and less error-prone (I mean why do we even care about release-related dependency during testing when these packages are not relevant) we should actually remove release-related dependencies from package.json and install them as a separate step in the release workflow, fixed to a specific version. We already do it like this in Go projects -> https://github.com/asyncapi/parser-go/blob/master/.github/workflows/release.yml#L50-L51

So yeah, when I started writing this issue, I did not think I'll end up describing 2 fundamentally different solutions for one problem....especially since in my opinion we should actually work on these 2 solutions anyway.

Thoughts?

@jonaslagoni might be also related to #797

@derberg derberg added area/javascript Specify what technical area given issue relates to. Its goal is to ease filtering good first issues. area/ci-cd Specify what technical area given issue relates to. Its goal is to ease filtering good first issues. labels Jun 23, 2022
@derberg
Copy link
Member Author

derberg commented Jul 4, 2022

@jonaslagoni @fmvilas @magicmatatjahu need your view on this one

@jonaslagoni
Copy link
Member

Node 12 is no longer maintained by Node, and IMHO we should just drop it....but this is bigger, we have same release and pr testing workflows everywhere, we need to agree on dropping Node 12 everywhere, and the ones that do not agree, will need to maintain their custom workflows 🤷🏼

Well, we cant stay at 12 forever anyway, might be a good time to switch 👍 It's hard to know how many this will affect as I don't think we have any metrics for who is using the tools in Node 12 environments 🤔

I think I made a fundamental mistake in 2020 when I designed the release pipeline by managing the dependencies through the devDependencies in package.json. In my opinion to make it easier in the future, and less error-prone (I mean why do we even care about release-related dependency during testing when these packages are not relevant) we should actually remove release-related dependencies from package.json and install them as a separate step in the release workflow, fixed to a specific version. We already do it like this in Go projects -> https://github.com/asyncapi/parser-go/blob/master/.github/workflows/release.yml#L50-L51

The only concern I might have with installing the release-related dependencies separately it is almost impossible to debug and work on locally (might never want to do it 🤔?). But if you think it's a better solution then sure 👍

@magicmatatjahu
Copy link
Member

magicmatatjahu commented Jul 5, 2022

Node 12 is no longer maintained by Node, and IMHO we should just drop it....but this is bigger, we have same release and pr testing workflows everywhere, we need to agree on dropping Node 12 everywhere, and the ones that do not agree, will need to maintain their custom workflows 🤷🏼

Let's not exaggerate the fact that someone is using v12. If there are problems it will be fixed on the repository side.

About semantic-release: just as Jonas said, installing semantic releases deps on the workflow side can be problematic due to debugging difficulties, other problems I don't see and I like that idea.

We should make some official announcement when we wanna switch to the new approach and new version of NodeJS (alongside with npm version) and adjust all repositories in 1/2 days.

EDIT: Should we transfer that issue to the .github repo?

@derberg
Copy link
Member Author

derberg commented Jul 7, 2022

About semantic-release: just as Jonas said, installing semantic releases deps on the workflow side can be problematic due to debugging difficulties, other problems I don't see and I like that idea.

actually, this is why I added it to package.json to follow the principle that on local I can reproduce......but for last 2 years it was not needed, and it is causing only additional complexity in the setup. And for local, someone should reproduce with https://github.com/nektos/act really.


I will leave this issue here, as it was mainly related to generator, failing tests and I want to have it there for people interested in reason why issues are in generator

I will create a new issue in .github describing what will be done and clearly communicate that we do not really enforce v12 deprecation, because anyone can really eject from official pipelines and have separate ones.

In the issue I will mention all maintainers from all the repos, to raise awareness and visibility

Any objections?

@derberg
Copy link
Member Author

derberg commented Jul 18, 2022

Some clarification, while working on creating an issue I realised that our global PR testing workflow tests only against 14 by default. Only in generator I used matrix in custom PR-testing workflow. So we drop support of 12 officially only here....which might not be needed if I do what I plan with release pipeline refactor 😅

I'll keep ya posted. First the refactor of release pipeline must go through.

@smoya
Copy link
Member

smoya commented Jul 18, 2022

More kinda related issues:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ci-cd Specify what technical area given issue relates to. Its goal is to ease filtering good first issues. area/javascript Specify what technical area given issue relates to. Its goal is to ease filtering good first issues.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants