Resurrection of the client-side infrastructure upgrade effort

[MariaAga]

I started updating to npm 8, but didn’t have the time to continue here: feat(root): update lock files to v2 / npm 8 by MariaAga · Pull Request #404 · theforeman/foreman-js · GitHub

[Shimon_Shtein]

From what I see webpack 5 requires node v10+, which is good, since we already use at least node 12.

I didn’t see direct requirement to change npm version, unless I am missing something.

@ekohl I understand the concern to upgrade the automatic testing to GH actions and Jenkins pipelines. Those changes will give us more flexibility to test the changes much faster. I am not convinced yet that it is a must-have in order to switch to webpack5, so I think we could leave this change to the next steps in the process.

@MariaAga, is there any component that will not run on npm < 8 after the upgrade to webpack 5? I want to understand when we should do the npm upgrade.

I think that at least at this moment, the next immediate step would be to change webpack to v5.
I don’t see any additional dependencies that have to be upgraded for this to happen.

My plan would be to downgrade the machine to NPM 6.14 and Node 12 (lowest possible), upgrade webpack to 5 and see if I can start the system and run tests on this setup. If it’s possible - I think it would be a good candidate for the first step.

@ekohl , let’s grab some time to talk about the test infrastructure unification and see when these steps have to happen and what benefits they bring to the table.

[Shimon_Shtein]

@ehelms , I am still not convinced at which step we want to upgrade our npm/node dependencies, but it looks like currently we have to postpone this change unless we want to stop supporting EL8’s default setup. EL8 defaults to npm 6, but the package-lock.json structure changes between npm 6 and 8, so if we modify our lock file, it will break EL8 builds.
Do we have to use the default npm version, or is it possible to use npm 8 / use node >= 16 (which already comes with npm 8)?

[MariaAga]

I wanted to update to npm 8 before doing the webpack update as there are overrides in npm 8 which might be helpful during the upgrade (overrides | package.json | npm Docs), and the lockfile v2 (which is created with npm 8) is backwards compatible with npm 6. But it is not required.

[Ron_Lavi]

Just wanted to make it clearer that React upgrade isn’t blocking us from Webpack upgrade,
we can still upgrade Webpack if we want before upgrading React, but we definitely should be moving forward also with React.

+1 for removing snapshot testing, but we still have a lot of unit tests and integration tests that are using Enzyme. Since I don’t see us re-writing most those tests in the near future,
we can follow the approach mentioned here:

• The TLDR is that we can have two jest.config.js
• run jest twice : on --config jest.config.js then on --config jest2.config.js
• In the example they use .spec2.js for new test’s files using react-testing-library (and react 18 under the hood)
• They keep their old .spec.js for old tests using enzyme (and react 17 under the hood) and migrate them progressively

And this will allow us to migrate to React 17/18

[ekohl]

@Shimon_Shtein

@ekohl , let’s grab some time to talk about the test infrastructure unification and see when these steps have to happen and what benefits they bring to the table.

I think the primary thing I would want out of it is a simple way to select the correct versions of NodeJS (and Ruby) for a certain Foreman version. Today you need to spend a lot of effort in many repositories.

Note that for Path to Ruby 3.0, 3.1, EL9 and Ubuntu 22.04 we’ll want the same thing.

[ezr-ondrej]

I don’t have a pony in the race much, but my thinking is, that this effort might be much easier when we would directly switch to webpack less javascript building, I know @Ron_Lavi already talked about it somewhere.

Given how much effort this upgrade is, I believe it would be better to switch to import maps right away.

More details on the new stack in Rails 7.0, if anyone would be interested: Ruby on Rails — Rails 7.0: Fulfilling a vision

[Shimon_Shtein]

IMHO we are too invested in react to do importmaps. Specifically we have react components composition, where we have sub-components coming from different plugins. I am not entirely sure that this would work well with sourcemaps, although I think it can be a nice addition to managing common dependencies. From reading DHH’s ideas around importmaps, he’s targeting JS-light applications for it and I don’t think Foreman still qualifies as such.

I do think that RedHat’s microfrontends framework can be a quite good helping tool, since it gives us exactly what we are missing: a composable client. In order to test it, the first step would be to upgrade webpact and react. Once we are there, we can either decide to ditch the whole webpack, or to continue to consume Javascrpt built by webpack with or without the microfrontends framework.
That is the goal of the current effort.

[Shimon_Shtein]

First version of the PR is out: Switch foreman to webpack 5 by ShimShtein · Pull Request #9646 · theforeman/foreman · GitHub
It runs, but the build time increased, so it can cause a timeout on cold dev server startup.
Also I can see that the react-app package is duplicated multiple times:

Next steps would be:

• Go over existing plugins and decide what’s still needed and what things are deprecated.
• Switch to webpack 5 in foreman-js package

[Shimon_Shtein]

A general question:
What would be the acceptance criteria for this PR?
Obviously we should be able to spin up a dev machine and compile the JS for packaging.
Any constraints for size or timing?

[jeremylenz]

@Shimon_Shtein

Any constraints for size or timing?

I’d say “don’t get any worse than we already are” is a reasonable starting point.
also, make sure it doesn’t break Katello and other important plugins

[Shimon_Shtein]

General update:
It looks like the best way to move forward would be to start WP5 configuration from scratch and see what we are missing there, instead of trying to adapt the current configuration and custom plugins. The reasoning is that the purpose of the plugins was to create module federation with older WP versions. Now that we can use WP5’s module federation, the need for the complex configuration is not there anymore.

foreman-js on WP5 dilemma:
Theoretically we can expose all the libraries under foreman-js as shared modules, and consume the same copy of the module in all foreman plugins. The downside of this approach is that we can’t tree-shake the library, effectively loading everything it brings into browser’s memory.
Alternative approach would be creating a framework that will reuse the same library version in all plugins during the compilation phase, but each plugin would “bake in” all the parts of the library it uses, and ship as part of its own module.

Additional reading for the dilemma: [Question] What's the best practices of shared dependencies? · Issue #2219 · module-federation/module-federation-examples · GitHub

[ekohl]

@Shimon_Shtein

Theoretically we can expose all the libraries under foreman-js as shared modules, and consume the same copy of the module in all foreman plugins. The downside of this approach is that we can’t tree-shake the library, effectively loading everything it brings into browser’s memory.

This was the whole design of the foreman-js library. From the start we knew we couldn’t tree shake it, since you don’t know what plugins are going to use.

I suppose this is also the reason that importmaps expose all module separate, then it’s up to the client to retrieve what it needs instead of some huge amount that you may or may not need.

@Shimon_Shtein

Alternative approach would be creating a framework that will reuse the same library version in all plugins during the compilation phase, but each plugin would “bake in” all the parts of the library it uses, and ship as part of its own module.

I think this is going to duplicate more than having a common library. Every plugin that uses webpack will pull in react and patternfly.

Overall I’d say start with foreman-js as a shared library (accepting we can’t tree shake) and then figure out a (long term) path to import maps to avoid webpack as much as possible. Relying on native browser functionality is IMHO still a better path forward, even if it’ll take a long time to get there.

In the medium term we should critically look at what foreman-js pulls in. If it pulls in multiple versions of the same library, then figure out if we can align those on a single (latest) version for example.

[Shimon_Shtein]

@ekohl

Overall I’d say start with foreman-js as a shared library (accepting we can’t tree shake)

Agreed. That’s the path I am taking right now.

The main obstacle I want to overcome right now is the ability to load the modules list dynamically.
By default the host application (the one that consumes other modules) should define in its configuration all the modules that it intends to consume. Since we don’t have the full list during the webpack build time, I want to understand how this list can be populated in runtime.

[Shimon_Shtein]

Update: there is an example of using dynamic loading of components here
Now I need to wrap this code into rails helpers and load the JS module from the plugin using the dynamic loaders.

[Shimon_Shtein]

Another update:
I am in the middle of adapting Foreman’s helpers to support the new set of webpack’s output.
Currently there is jsbundling-rails way, where first all the webpack assets are compiled and then they are served as static JS files from the rails’ assets pipeline. This puts some restrictions though, since dynamic links inside webpack generated JS will not work.
As a first step I will try to disable the fingerprinting, to see that everything actually loads correctly. Once I have a running Foreman instance, I will start adapting the solution to production-ready environment.

[MariaAga]

Hi everyone, Shim and I made a working version of Foreman with webpack 5! Please help us test it and make any improvement suggestions to the new configuration.

We have successfully achieved a stable configuration of Foreman and its plugins using Webpack 5. In this revamped setup, we’ve embraced module federation to streamline the loading process. Because we are using module federation, each plugin will be built separately and will be dynamically loaded. The configuration for each plugin will be the same, and is located in foremans webpack.config.

One of the major changes in this configuration is the shift towards utilizing files from the public folder instead of relying on the webpack dev server to serve them. This change serves two primary purposes: first, it eliminates the need to run a webpack dev server for each plugin, and second, it simplifies the loading process within Ruby. After this change, every plugin will have a folder in public/webpack with its webpack assets (@packaging). This change also removes the webpack-rails gem.

This transition has necessitated adjustments to the loading order and the implementation of a few workarounds:

1. Since the plugins are loaded asynchronously, we have introduced the loadPlugin global event that will be fired once all the plugins are loaded. Which means, JavaScript scripts/files loaded with content_for(:javascripts) will now load only after plugins are loaded.
2. The react_component will search for its component only after all plugins and JavaScript scripts have loaded, otherwise it might not find the component.
3. In Katello: We’ve made crucial changes to the bastion code to ensure it loads in the correct order. Without this adjustment, Angular pages may not function as expected. Specifically, this resolves these errors:

• BASTION_MODULES not being defined and issues with angular.element(document).ready, which is no longer effective due to dynamic script loading.
• angular.bootstrap can only run after all angular.module(... were run.
• Unknown provider: $rootScopeProvider ← $rootScope making angular pages not load

• webpack_asset_paths(plugin_name, extension: 'js') for Javascript files should not be used anymore, and plugins that use it should replace it with content_for(:javascripts) { webpacked_plugins_js_for(plugin_name) }. For the next Foreman version we will override the javascript_include_tag function to make sure plugins are loaded correctly.
• webpack_asset_paths for CSS files should not be used as the css will already be loaded.

5. Since we no longer run webpack as a server, we will remove webpack_dev_server and webpack_dev_server_https options.

We are sharing the react_app/components/HostDetails/Templates folder for a specific reason. Without this sharing, each plugin creates its own file for webpack/assets/javascripts/react_app/components/HostDetails/Templates/CardItem/CardTemplate/index.js. This isolated approach prevents the React Context from being properly passed to the plugins, resulting in crashes on the host details page’s details tab.

Other changes:

1. SimpleNamedModulesPlugin was a simple version of webpack 3 SimpleNamedModulesPlugin, we removed it and we are using optimization:{ moduleIds: 'named',} as recommended by webpack 5 instead, @packaging will that work for you?
2. Webpack 5 no longer automatically includes the buffer package, so we need to import it manually in webpack/assets/javascripts/react_app/common/globalIdHelpers.js.

While we’ve achieved significant improvements with this configuration, we’re aware of a few challenges that still need attention, now or in the future:

1. Lengthy webpack compile times (sometimes up to 5 minutes).

2. Increased JavaScript file sizes.

3. Longer page loading times for certain pages (for example: /foreman_tasks/tasks, /products).

Relevant PRS to use the new configuration:
Foreman: webpack 5 by MariaAga · Pull Request #9834 · theforeman/foreman · GitHub
Katello: webpack5 by MariaAga · Pull Request #10735 · Katello/katello · GitHub

To ensure a clean run with Webpack 5, please follow these steps in the Foreman folder:

rm -rf node_modules public/webpack/ public/assets/
npm install

And then:

Run webpack and rails separately:

npx webpack --config config/webpack.config.js --watch
bundle exec foreman start rails

Or together:

bundle exec foreman start

As we move forward, our goal is to enable plugins to consume Foreman as a module via package.json while leveraging the expose and shared features in the webpack’s module federation configuration. We believe this approach will help reduce JavaScript file sizes and prevent Foreman CSS from being duplicated in every plugin, mitigating issues related to CSS file size and compatibility with older plugins.

[ehelms]

@MariaAga

One of the major changes in this configuration is the shift towards utilizing files from the public folder instead of relying on the webpack dev server to serve them. This change serves two primary purposes: first, it eliminates the need to run a webpack dev server for each plugin, and second, it simplifies the loading process within Ruby. After this change, every plugin will have a folder in public/webpack with its webpack assets (@packaging).

Can you expand on this a bit? Is this only meant to indicate changes in the development environment set up? What will be in this folder? If yes, it would bring some continuity but I am curious then what happens in production where we already use this folder structure to deliver compressed and minimized assets. For example, every plugin in production has a public/webpack/<plugin name>/ folder with these assets.

@MariaAga

SimpleNamedModulesPlugin was a simple version of webpack 3 SimpleNamedModulesPlugin, we removed it and we are using optimization:{ moduleIds: 'named',} as recommended by webpack 5 instead, @packaging will that work for you?

I don’t know how this affects packaging and production set up to be honest. Can you help explain what you think it would affect?

@MariaAga

Run webpack and rails separately:

npx webpack --config config/webpack.config.js --watch
bundle exec foreman start rails

Based on this and the above comments about public/webpack, do I have it correct that this means:

• Rails serving up the assets now in development
• npx webpack is just a file watcher that initiates a compile when things change
• Using Apache in front of a development setup will now correctly work and mimic production better

@MariaAga

• Lengthy webpack compile times (sometimes up to 5 minutes).

Is this only on first generation or this means in development set up a change to a file might trigger a 5 minute wait while it compiles?

[MariaAga]

@ehelms

What will be in this folder?

In production:

In development:

@ehelms

I don’t know how this affects packaging and production set up to be honest. Can you help explain what you think it would affect?

We couldnt see why it was excatly needed which is why I tagged packaging, as it looks like a solution made for packaging.

@ehelms

Based on this and the above comments about public/webpack, do I have it correct that this means:

• Rails serving up the assets now in development
• npx webpack is just a file watcher that initiates a compile when things change
• Using Apache in front of a development setup will now correctly work and mimic production better

Yes

@ehelms

Is this only on first generation or this means in development set up a change to a file might trigger a 5 minute wait while it compiles?

If a file changes it will only trigger 1 webpack recompile, so for example if I change a file in Katello, only Katello will recompile and not the other plugins so it will take less time. Currently we don’t have a solution for this, we think that exporting Foreman to a module will help with that but did not do any testing with that.

[ekohl]

@MariaAga

@ehelms

I don’t know how this affects packaging and production set up to be honest. Can you help explain what you think it would affect?

We couldnt see why it was excatly needed which is why I tagged packaging, as it looks like a solution made for packaging.

If I recall correctly, it’s sort of related to packaging. Without the plugin the files would be stored by their absolute path and when you add plugins to that it would all break. You would see /buildroot/BUILD/... as paths, because that’s what Koji uses. So rather than that, it would be stored by their module name. It’s very likely that by now webpack has defaulted to a better solution just because they now properly support modules, but I’m just going by memory and not looking anything up.