openstreetmap / openstreetmap-website

The Rails application that powers OpenStreetMap
https://www.openstreetmap.org/
GNU General Public License v2.0
2.17k stars 911 forks source link

Resolve problem with js-cookie 3.x and node engines #3275

Open gravitystorm opened 3 years ago

gravitystorm commented 3 years ago

js-cookie 3.x has a dependency of nodejs >= 12. However, Ubuntu 20.04 ships with nodejs 10. We generally target the latest Ubuntu LTS, and this is also what we use for CI.

The upgrade to js-cookie 3.x was in 6feb125f4f0393bcfd319c426adb0b5f7e3c1c0f and as far as I'm aware was just for dependabot reasons rather than any required feature or bugfix. 102f3a6668665774f069a8116a251a28c3c5c0cc patched bin/yarn to ignore engines, and I can make a PR to the Dockerfile to do the same there.

I just wanted to open this discussion to check if ignoring the engines is the right approach, or if there are other workflows that might break (e.g. for other developers). As far as I can tell, for the js-cookie node module we're just using yarn/nodejs for code distribution and not otherwise using node for anything clever. But we use other node modules where we actually run the code in node (e.g. eslint in development, or terser in production) and where the engine used is more likely to be important. By ignoring the node engines, are we setting ourselves up for a problem later on e.g. when eslint or terser uses features from node 16 or 18 and we don't realise because we've disabled the engine check?

So options include:

tomhughes commented 3 years ago

Pinning js-cookie is no long term solution unless upstream are going to maintain that branch - if a security issue is discovered and only fixed in 3.x we might be forced to upgrade at short notice.

You could argue it's a bit bizarre for js-cookie to require a given node engine given it's code that is only ever likely to be executed in a browser and not server side with node...

tiziodcaio commented 2 years ago

I think the best thing to do is to upgrade node, the version 10 is in EOL... I cannot understand why focal fossa still uses this version now that the dependencies' requisites are higher

tomhughes commented 2 years ago

We don't have a choice about node versions - we get what the operating system give us.

We're not going to start installing node from upstream just for this!

tiziodcaio commented 2 years ago

Why not using the tag rolling which provides ubuntu 21.10

tomhughes commented 2 years ago

Our production servers run on LTS releases, so currently Ubuntu 20.04.

tiziodcaio commented 2 years ago

So we'd wait a month for 22.04 for this PR and also #3515

tomhughes commented 2 years ago

Well we wouldn't install it immediately - the LTS version won't actually technically be released until the .1 release of 22.04 later in the year though we often start to upgrade before that.

We will definitely start a program of upgrades are some point later in the summer though.

gravitystorm commented 2 years ago

We're not going to start installing node from upstream just for this!

Perhaps not. But I think we'll need to address the issue of Node versions at some point this year. Ubuntu are, slightly bafflingly, about to release 22.04 with Node 12, which is EOL at the end of this month(!). Ignoring Node 14, Node 16 has also been available for over a year, and is the only current LTS version that will still be supported, just, when 24.04 is released.

So I think when we upgrade our Ubuntu requirements to 22.04, we'll need to recommend installing Node (probably 16, maybe 18) from an external source, since Node 12 will be already put out to pasture by that point. I use the NodeSource APT repo elsewhere, and it works well.

I really wish Ubuntu were a bit less daft with their Node versioning policy.

tiziodcaio commented 2 years ago

Or maybe using github ubuntu docker images which are more updated (for example they ship node 16)

tomhughes commented 2 years ago

No we are not going to be using github docker images in production!

tiziodcaio commented 2 years ago

No we are not going to be using github docker images in production!

Ok as you wish, your choice

tiziodcaio commented 2 years ago

Or maybe using github ubuntu docker images which are more updated (for example they ship node 16)

Or maybe use debian I don't know... Because ubuntu nodejs version management is not being the best at the moment, as I see

JesseWeinstein commented 2 years ago

At least for me, on current master branch, eslint fails to run in the docker build due to the old version of node. I'm likely doing something wrong, but just in case I'm not, I wanted to mention it here.