Update Node and npm to a more recent version

[smoya]

TL;DR;

• Node and Npm version should be updated to a more recent one, as we are mostly developing with newer node and npm versions, also because committed lockfiles are version 2.
  Lockfile version 2 is available starting at npm v7, However I think we could update it even to a more recent version.
  For example, Node v16, that includes npm v8).
• npm install command here should be replaced by npm ci so we avoid side effects (no modification of package-lock.json).

Any strong reason to not do this?

Long version if you are not very convinced

The following is a real use case of a bug in the produced image. It took me like an hour to figure out the "bug" (thanks to @magicmatatjahu for pairing!).

The very first Docker image of server-api was built and pushed to Docker Hub by our GH workflow action if-nodejs-release.yml.

Unfortunately, after trying to deploy it to a K8s cluster, we found out that the container was failing due to a missing node module: Error: Cannot find module '@babel/core'. By pulling the same image locally, we discarded the problem was in K8s but in the image itself.
The surprise is that, whenever that image is built locally (e.g. in my computer), the container works like a charm.

But why?

After diving (literally, using dive tool) into both Docker images (the one created by CI and the one created by myself locally), I noticed there was a big difference on a particular Docker layer. In particular, in the one created by COPY package* ./.

As you can see, the package-lock.json filesize differs. The one created by CI is much smaller (554 kB VS 1.2 MB).

After reading both files, I noticed that the package-lock.json from the image created by CI was a version 1 ("lockfileVersion": 1), however both the one created locally and the one available in the repository source code are version 2 ("lockfileVersion": 2).

But how can be possible if dependencies are installed inside docker? See Dockerfile. Could be a bug with Docker? The answer is no.
Dependencies were not built at the time RUN npm ci was executed, by way before. In particular, in the GH workflow action that precedes this. Inside Docker image we only run npm ci, which is intended for just using the pacakge-lock.json as the list of dependencies to install without modification.

This line is running a npm install command, so all dependencies are installed. Is the GH action running an old npm version that doesn't support lockfile version 2? Well, considering that the image where we run is ubuntu-latest, the npm version should be 8.1.2 (according to this). But we are not using that NPM (nor node) executable, but rather the one installed in the previous step, which is:

• Node v14.18.3
• NPM v6.14.15

.

The reason why the npm lockfile version is 1 in CI is because npm install that ran in the workflow with NPM v6.14.15 doesn't understand lockfile version 2 fully, so it literally tries to do the best (which seems it's not the best for us). As the warning says:

❯ npm install
npm WARN read-shrinkwrap This version of npm is compatible with lockfileVersion@1, but package-lock.json was generated for lockfileVersion@2. I'll try to do my best with it!

Due to that missing dependency (@babel/core) that npm couldn't install, the app is broken.

Related issues

• [New Doc 📑]: specify which version of npm is required to run npm install. parser-js#427

[magicmatatjahu]

Thanks @smoya for creating that issue! :) We can have one problem, I'm not sure but I remember you @derberg had problems bumping up NodeJS to version 14 on generator. Am I right? Will it be that big problem bumping NodeJS up to version 16?

[smoya]

@derberg do you know if there is any issue/blocker we should know about that stop us from moving forward on updating Node and npm?

[derberg]

in generator all should be fine "in theory" as issues for version 14 were fixed few months ago.

I'm more interested in how you see it? switching from 14 to 16 or rather test on both, IMHO it should be both

[derberg]

I'm more interested in how you see it? switching from 14 to 16 or rather test on both, IMHO it should be both

ping @smoya
The problem is that node 14 comes with npm 6 and node 16 with npm 8 already. So we start testing on 16 too but dropping 14 testing is like dropping support for it, no? In perfect world we should run tests against all supported versions

[smoya]

I'm more interested in how you see it? switching from 14 to 16 or rather test on both, IMHO it should be both

ping @smoya The problem is that node 14 comes with npm 6 and node 16 with npm 8 already. So we start testing on 16 too but dropping 14 testing is like dropping support for it, no? In perfect world we should run tests against all supported versions

We use Lockfile version 2, and it is only available on npm v7 or greater. So yes, we drop support for node versions lower than 16 (it doesn't mean our software won't work on those versions, but we just don't give support).

[derberg]

Lockfile v2 will still work with older npm:

npm WARN read-shrinkwrap This version of npm is compatible with lockfileVersion@1, but package-lock.json was generated for lockfileVersion@2. I'll try to do my best with it!

Version of used Node should not depend on version of used npm. Many people in Nodejs ecosystem do not even use npm but yarn or pnpm

[magicmatatjahu]

@derberg

Lockfile v2 will still work with older npm:

Yeah, it's true, but in our case with server-api we had problem when npm (with support for v1) installed wrong dependencies from package-lock 😄 I don't remember which package was installed wrong but there was such a problem. It would best to bump up the node with npm together so there are no problems.

[smoya]

Lockfile v2 will still work with older npm

As @magicmatatjahu mentioned in the comment above, this is not 100% right. And that's the whole reason why this issue is opened. You can read the long version and will understand the problems we can introduce without knowing about them.

[derberg]

brace yourself, a stupid question is approaching 😄

I still think that the node version and npm version discussion should be split.

I understand that there are some strange issues when installing with npm v6 or v7 or v8. But all of this is related only to npm and has nothing to do with node version. As I wrote above, npm is one of many dependency management tools in JS ecosystem, just most widely adopted as it was first, like facebook 😄

so why would we force node 16 on all the projects, just because in server-api we had installation issues because it had package-lock v2 but was run through npm 6.

I think it is ok if we just make that all the pipelines use npm > 6, through setup-node action that we already use (they do not support it yet actions/setup-node#529) or by adding a specific step to install given npm first.

And in server-api you should just enforce given npm version with https://github.com/asyncapi/generator/blob/master/package.json#L12 and that is it.

Thoughts? or am I missing something? but from description and comments I do not identify any issues with the runtime, only installation

[smoya]

Thoughts? or am I missing something? but from description and comments I do not identify any issues with the runtime, only installation

Each NodeJS release is bundled with a particular NPM version. Agree you can change that, but using too older or too newer versions of NPM could cause issues as well.
The ideal is to use the same version bundled with your NodeJS version, available on this list https://nodejs.org/en/download/releases/.

That's why, NodeJS 15 is the minimum version for using NPM 8. In fact, I would recommend using NodeJS 16 because is a LTS.

[smoya]

There is an extra issue to consider here. Keeping lockfileVersion to 2 it's impossible unless we accept a change such as #187. The reason is that all current GH actions are converting back from lockfileVersion 2 to 1 every time they run, and the if-nodejs-release workflow commits the changed package-lock.json to the released branch 🤷

[smoya]

Another option would be to enforce all projects to stay with Node v14 + NPM 6, meaning lockfileVersion: 1 until we consider updating to a more recent version bundle makes sense (adoption as main driver here).

[derberg]

Each NodeJS release is bundled with a particular NPM version. Agree you can change that, but using too older or too newer versions of NPM could cause issues as well.
The ideal is to use the same version bundled with your NodeJS version, available on this list https://nodejs.org/en/download/releases/.

I totally get it. But I do not think it is a common practice/knowledge that "if you decide to use lockfileVersion: 2 it means you stop supporting Node with a version that bundles npm with v2"

The reason is that all current GH actions are converting back from lockfileVersion 2 to 1 every time they run, and the if-nodejs-release workflow commits the changed package-lock.json to the released branch 🤷

you are right, but not the release workflow, but the bump workflow. Release workflow do not make changes to repo, the bump of version is 😞

Another option would be to enforce all projects to stay with Node v14 + NPM 6, meaning lockfileVersion: 1 until we consider updating to a more recent version bundle makes sense (adoption as main driver here).

Ok, so the lowest LTE is v14, latest LTE is v18. With NPM mapping looks like 👇🏼 and I'm only considering LTE

Node	npm	lockfileVersion
14	6	v1
16	8	v2
18	8	v2

What I think we agree on:

• using v2 may cause failures on node 14
• telling Node 14 folks to upgrade npm manually is not good DevX
• issue is only with v2, and when someone uses v1 on node > 14 then all is good

What we need to agree on:

• we should not enforce v2 on projects that want to support Node 14
• CI should work this way:
  • if the project uses v2 then tests should run only starting from Node 16 and it should be documented that Node 14 is not supported. This is a breaking change
  • if the project uses v1 then tests should run starting from Node 14

After all these discussions, I personally think that the best practice is if maintainers work always on lowest LTS, not highest

[jonaslagoni]

While trying to upgrade Modelina for dotnet-nats-template and ts-nats-template I ran into some issues where the dependencies didn't get installed correctly, and I was using node v14.20.0 and npm 6.14.17 with lockfile v1. I am not 100% sure why I had this problem, but it was related to the new parser version, where spectral and ajv clashed, or something in that order.

@magicmatatjahu then suggested to try and upgrade and use a newer lockfile version, so I upgraded to npm 9.3.1 and removed node_modules and package.json file and re-ran npm i. Now everything seems to be working correctly 🤷 I still have yet to upgrade the release workflow to use the new npm version, so for now it's only been tested locally.

So using lockfile v1 for node 14, might not work going forward. At least when it comes to similar scenario 🧐

I will get back to you if I encounter any more issues with dependencies...

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity 😴

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience ❤️