NodeJS 18 compatibility and webpack 5 upgrade

Hi! I started working on some needed updates to the Javascript infrastructure.

I am planning on doing 3 updates in the following order: npm → React → Webpack, and will go into details on what updates I’m doing and why.

General why

Updating now will make it easier to maintain and update later on.
Updating usually requires changing some of our code, updating it now when everything works fine means we can do it carefully and take our time.

npm 8

The main change here is in foreman-js and everyone that doesn’t contribute to foreman-js can keep using which npm version they like.

Unlike in foreman core and plugins, in foreman-js we save the package-lock.json file for stability.

Currently we install foreman-js using npm v6 (that’s why the package-lock.json is version 1: package-lock.json | npm Docs)

When installing using npm7+ the lock file is built in version 2 which is backwards compatible to v1 lockfiles.

Implementations:

Why?

Potential/current issues:

  • Min node version that we can use is 12 for this version of npm and some parts of packaging use node 10
  • NPM peerDependencies issue
  • other broken peer dependencies

Solutions:

React 18

We use 16.9.

Previous update to 17 had to be reverted because pf3 tooltips stopped working: Fixes #30787 - upgrade js vendor to 8.3.0 by MariaAga · Pull Request #7963 · theforeman/foreman · GitHub

Solution: remove all uses of overlayTrigger Refactor #34713: Remove PatternFly 3 OverlayTrigger - Foreman

Changes mostly foreman-js and react-dom use in foreman core

Update prs(WIP):

Why?

  • Automatic Batching: Batching is when React groups multiple state updates into a single re-render for better performance.
  • More React things to use: React v18.0 – React

Potential/current issues:

  • React-dom changes
    Solution:
  • delete most of our react-dom use here: https://github.com/theforeman/foreman/pull/9212
    * update the rest of the react-dom uses in this file is much less complicated once the most is deleted
  • Packages have react 16 as peer dependencies
    Solution: npm8 will automatically install peerDependencies

Webpack 4

We use 3 for a very long time as updating it has been challenging.

Really basic draft:

https://github.com/theforeman/foreman/pull/9210
Changes only foreman core

Why?

Potential issues:

  • No one is sure how we build the ui
  • Using plugins - when using webpack 3 we could not use tree shaking and minifications because it will cause plugins to stop working.
  • Katello
  • Common chunks plugin is no longer used
  • In the draft pr, the –analyze shows that the bundles are ~x2 larger than before the PR
6 Likes

Thanks a ton for this write up Maria!

A few details and questions, mostly from an infra/packaging PoV.

Today, we use a mixture of node 10 and node 12 across our targets (Debian uses 12, EL7 uses 10, EL8 uses 12).
My understanding is that nothing prevents us from upgrading to node 14 today, besides the fact that someone would need to do the actual changes to our infrastructure. Is that correct? And if it is, is (well, should?) it also correct for Foreman 3.1/3.2?

I am asking because our Debian builders are currently set up to use the node packages from Index of /node_12.x/, regardless which Foreman version we’re building for. If we could just switch that to Index of /node_14.x/ for a test-ground, that would be awesome.

For RPM, we have more granular control, and could switch EL8 nightly to node 14 rather easily. I’d prefer to just drop EL7 instead of fixing it there too (this should happen “soon”).

1 Like

Yes, we can use node 14 now. as for previous versions of foreman I did a quick check (running npm i on foreman 3.1) and it should be fine as well.

2 Likes

I’ve added support puppet-katello_devel to install EL8 dev environments with nodejs 12 or nodejs 14.

For anybody interested in trying this, see the example vagrant box definition in Suggest --katello-devel-modulestream-nodejs installer arg by wbclark · Pull Request #1508 · theforeman/forklift · GitHub

3 Likes

I am going to attempt to summarize what I think my understanding of the situation is so that other’s can correct me if I am in error and propose a way to unstick things.

If I gather this correctly, the ordering of updates means we need to move to NodeJS 14 to open up the ability to perform other dates in the stack which would then unlock moving to NodeJS 16+.

I would propose then, given the tricky bit is needing NodeJS 14 with NPM 8 on EL 8, that we package NPM ourselves on EL for a period of time such that we can make this transition. We would then, when applicable, revert to use the NPM from the dnf modules.

I’d be happy to help facilitate this effort if we agree what steps are needed to bridge the gap.

2 Likes

@ehelms Just wanted to bump this. I think it’s important that we move forward with this ASAP. The steps you outlined make sense to me. Let me know if I can help in any way.

Did a little playing around, building just NPM will be tricky as NPM is not simply it’s own package but built as a sub-package from Node – not impossible, just trickier I think.

One item I was trying to better understand is what is the blocker to updating Webpack to version 4 or 5?

Thanks for the help offers, I’m going to step back a bit from working directly on it.
The last issue I had with Webpack 4 in the pr are that each css file that had import weighted a lot more than it should. and also that it was not tested at all in Katello so there might be issues there.
https://github.com/theforeman/foreman/pull/9210

One thought I’ve had is what happens when we merge Rails 7. That has alternatives which may mean we can drop webpack altogether instead. @Ron_lavi did some investigation in Using React with importmaps on Rails 7 but I’m not sure this is a realistic plan.

I also wonder if switching to a more modern build tool like https://vitejs.dev/ is worth investigation.

Given the churn in tooling in the JS ecosystem I’d actually try to keep it as simple as possible and follow Rails’ guidance. In the end we are still a Rails application that has additional Javascript, not a pure JS application.

With Foreman 3.10 we’ve updated to NodeJS 18 compatibility using legacy peer dependencies and switched over to webpack 5.

@MariaAga can we consider this RFC to be complete or do you also consider React 18 to be within scope?

1 Like

Yeah I agree, I think this is complete and the React 18 can be separate

1 Like

Great. I’m moving this to the RFC category so I can mark it as resolved. I’ll also update the title to reflect the scope.

1 Like