Fixes #37134 - upgrade to node 18 and npm 8

[MariaAga]

No description provided.

[ekohl]

For context, this is blocking EL9 so if we can't get this done for Foreman 3.10 we'll need to postpone that effort to 3.11. I'd really like to avoid that.

[MariaAga]

If webpack 5 is not reverted I dont see a reason for this to not be merged really soon. Looking at the test failures now.

[MariaAga]

@ekohl do we need npm8 for EL9?

[ekohl]

@ekohl do we need npm8 for EL9?

Most likely In EL9 they've packaged NPM 8 and we're not downgrading that to 6. So if we need NPM (and I'm pretty sure we do), we need to be on NPM 8. It is acceptable to also move our other platforms to NodeJS 16 and NPM 8 if needed (and I think we can't support both).

However, looking at the Foreman 3.10 schedule we'll have stabilization week from 2024-02-12 - 2024-02-17. The week before we have cfgmgmtcamp where both @evgeni and I will be.

Essentially that means we need to get this and the needed rebuilds done next week. I'm skeptical we can make that work, unless @ehelms steps in. Even then, getting reviews will be hard.

[evgeni]

Yeah, npm8 is what we get via CentOS packages and we can't easily avoid that.

[ehelms]

For clarity sake, on EL 8, we get the following:

Installing:
 nodejs                                                   x86_64                                         1:16.18.1-3.module_el8.7.0+1234+1d8589d9                                                     appstream                                          12 M
Installing dependencies:
 brotli                                                   x86_64                                         1.0.6-3.el8                                                                                  baseos                                            323 k
 openssl                                                  x86_64                                         1:1.1.1k-9.el8                                                                               baseos                                            737 k
Installing weak dependencies:
 nodejs-docs                                              noarch                                         1:16.18.1-3.module_el8.7.0+1234+1d8589d9                                                     appstream                                         9.3 M
 nodejs-full-i18n                                         x86_64                                         1:16.18.1-3.module_el8.7.0+1234+1d8589d9                                                     appstream                                         8.0 M
 npm                                                      x86_64                                         1:8.19.2-1.16.18.1.3.module_el8.7.0+1234+1d8589d9                                            appstream                                         2.0 M

If this is just a question of iterating over packages for rebuilds, I can certainly help grind through that. I would need more clarity on if theforeman/foreman-js#404 is required and if that's the only property affected by this change? IIRC we do not store the lock files for any other repository.

[ekohl]

When I tried I could actually install it without forcing legacy peer dependencies. Are you sure it's still needed?

[MariaAga]

When I tried I could actually install it without forcing legacy peer dependencies. Are you sure it's still needed?

did you run npm test or compile webpack? for me tests were failing as they could not find test packages, and pf5 was chosen over pf4

[ekohl]

did you run npm test or compile webpack? for me tests were failing as they could not find test packages, and pf5 was chosen over pf4

No, I didn't. I just assumed that the initial error (plainly refusing to install at all) was the only error.

Just to be clear, I'm not opposed to using it for now if it gets us on NodeJS 16 faster.

[MariaAga]

Should we just do the jump and update to node 18 instead?

[ekohl]

theforeman/foreman_ansible#693 (comment) and theforeman/foreman_discovery#621 look related, the rest of the failures look unrelated.

#10045 should resolve those. They also happen with current NodeJS.

[ekohl]

All the plugin tests now pass. That means there's only the NodeJS 14 situation left.

We plan on shipping EL8 with NodeJS 14 for Foreman 3.10 so it will be nice to testing that. Otherwise you may pull in dependencies that are Node 14 incompatible.

That makes me wonder: why is this snapshot changing? Are there also effects on the end user because of this? If not, what is the test really proving?

[MariaAga]

That makes me wonder: why is this snapshot changing? Are there also effects on the end user because of this? If not, what is the test really proving?

Node has changes how dates are in strings, so instead of , there is at now, but the date is the same and thats what we test.

[ekohl]

Digging a bit deeper, it's nodejs/node#45945 but I think at runtime toLocaleString uses the browser API, not NodeJS. So it doesn't really matter for end users. Though it does matter for our tests.

Is there a way to provide a NodeJS version dependent snapshot? Would that make sense?

[MariaAga]

We can probably do that with setting different paths for snapshots for different versions. Are we planning to test and actively support node 14 or just leave it as an option?

[ekohl]

Are we planning to test and actively support node 14 or just leave it as an option?

As I said in #10016 (comment), we'll have it at least for Foreman 3.10 so I'd prefer to have testing for it too.

[MariaAga]

updated snapshots to be conditional

[ofedoren]

@MariaAga, needs rebase :/

[ekohl]

This should be both 14 and 18 now:

Suggested change

	"node": ["18"]
	"node": ["14", "18"]

[ekohl]

Recently I noticed we also have https://theforeman.org/contribute.html#SupportedNode.jsversions and I realized we also need to update that for Ruby. Isn't duplication great?

[ekohl]

Waiting for the container build to finish, but I think this is ready to merge now.

[ekohl]

@evgeni we should pick this to 3.10 too, right?

[evgeni]

@evgeni we should pick this to 3.10 too, right?

Given we publish EL9 packages that use NodeJS 18 it would be formally correct to do that, yes.

[ekohl]

642f772