RFC: Changing how we handle Webpack Building

[ehelms]

This looks promising and the write-up is very detailed, much appreciated! Would it be much work to do a POC within Foreman?

[amirfefer]

Another important thing, with today’s architecture, jest requires a mock for each and every component we use from foreman core (foremanReact alias), I hope that using meta packages would prevent that.

[John_Mitsch]

Would it be much work to do a POC within Foreman?

Sure, I can give it a shot and will let you know.

Another important thing, with today’s architecture, jest requires a mock for each and every component we use from foreman core (foremanReact alias), I hope that using meta packages would prevent that.

That would be nice, it seems a meta package would remove a lot of the package duplication that we need for testing in foreman plugins

[Walden]

Thanks @John_Mitsch this is something that we were looking at working on this iteration but glad to see that you got to it first. Even if using foreman as an npm package doesn’t fix the build issues it will definitely help with testing and de-duplication of packages.

We were planning on just referencing foreman from the katello package.json via a github url rather than using the file approach. What do you think of that?

[ekohl]

Perhaps it’s more of a downstream problem, but it also relates to our branches. It means every plugin must refer to right branch. Currently plugins like foreman_ansible are built against multiple foreman releases at once. Keeping this flexibility is very important IMHO.

So the question is: how do we find the right sources in an offline environment? Note we do control the build environment.

[John_Mitsch]

We were planning on just referencing foreman from the katello package.json via a github url rather than using the file approach. What do you think of that?

I don’t know enough about the build process to say for sure, I used the subfolder approach as it was easy to demonstrate.

Sounds like it would have to work in an offline environment like @ekohl says. Maybe the build team can clarify their criteria around this?

[Walden]

So the question is: how do we find the right sources in an offline environment? Note we do control the build environment.

An idea I had for the build machines was hosting a local copy of the node registry within the build environment and utilizing a package.json lock file. I’m not sure of the limitations and capabilities of the build environment so I don’t know if this is a feasible option but I do wonder if this could alleviate the need to package node packages as RPMs.

[ekohl]

An idea I had for the build machines was hosting a local copy of the node registry within the build environment and utilizing a package.json lock file. I’m not sure of the limitations and capabilities of the build environment so I don’t know if this is a feasible option but I do wonder if this could alleviate the need to package node packages as RPMs.

I don’t think that’s going to work. All RPMs are built in a container without any network access at all. For reproducible builds you need to provide all release artifacts at build time AFAIK, but people can correct me if I’m wrong.

It is also very useful to be able to do local RPM builds so you don’t want to rely on something within Koji. This allows the community to build their own plugin RPMs that are closed source. We know ATIX (cc @x9c4) also rebuilds RPMs for their product, like how RH does downstream builds.

[ekohl]

Currently we have some protection against issues like https://github.com/dominictarr/event-stream/issues/116 in the packaging process (even though we don’t do a real audit of the packages). IMHO it’s a huge feature that we don’t pull packages from NPM but rather from a trusted source. I would like to see how the new process would protect us from malicious code and attacks.

[Walden]

These are good points, thanks for the reply!

[ekohl]

Another point of concern is tree shaking. I’m not sure if it’ll be a problem, but I think the vendor bundle should not be stripped of unused functions because you don’t know what a plugin can rely on. Since this is something that won’t happen in development, I’m not sure how we can properly detect if it’ll be a problem.

[sharvit]

For the last weeks I tried to solve this issue by creating a sub-package @foreman/vendor that contains all the necessary npm-packages core and plugins needs.

core branch: https://github.com/sharvit/foreman/tree/feature/webpack-share-plugin
katello branch: https://github.com/sharvit/katello/tree/feature/webpack-share-plugin

How it works

Created a new sub-package in core that exports all the relevant npm-packages.
To import them you will need to do import React from '@foreman/vendor/react';
It mean we can have releases for @foreman/vendor with versioning and we might create rmp for @foreman/vendor instead for each node module.

Migrate imports
To migrate your code to the new imports, simply run

npm run vendor:migrate

Add a new npm-module
Created a generator script that do that automatically, in some cases the end results need adjustments.

npm run vendor:add classnames
# multi
npm run vendor:add classnames patternfly-react jed

Where i’m blocked atm

When I add katello to foreman and run npm test I get many of those errors:

Invariant Violation: Element ref was specified as a string (inner) but no owner was set. This could happen for one of the following reasons:
    1. You may be adding a ref to a function component
    2. You may be adding a ref to a component that was not created inside a component's render method
    3. You have multiple copies of React loaded
    See https://fb.me/react-refs-must-have-owner for more information.

I’m pretty sure I have multiple copies of React loaded .

Your are more than welcome to play with it and let me know what do you think.

[sharvit]

More updates, let me post a discussion we had internally:

Jan 18

Hi folks,

Last week Eric, Ewoud and I met to speak about webpack packaging to find a suitable solution for a long-lasting issue.

Here is the summary I came with, it’s all from my memory so feel free to add anything I forget:

1. sub-packages - we all agree it would be beneficial for core, plugins, and packaging to create consumable sub-packages in the core. Those sub-packages should be versioned correctly and consume by core and plugins.

2. @foreman/vendor - package to manage and consume production npm-dependencies.

3. @foreman/vendor should be built into vendor.js without tree-shaking .

4. Potentially, we should be able to create only one rpm from @foreman/vendor instead rpm for each npm-dep.

5. Can create linting rules that block developers from importing packages directly when they exist in the @foreman/vendor.

6. @foreman/dev, @foreman/test, @foreman/build - packages that manage dev/test/build dependencies and supply tools for those environments.

7. Doesn’t necessarily mean they should be separate packages, can be done in one package.

8. @foreman/lib - Instead aliasing the foremanReact for imports, it would be more beneficial to have a documented sub-package with everything we want to share with plugins so we stay controlled about what plugins can use and what not. (tfm functions, components, helpers…).

9. How core should consume plugins? We all agree it’s very odd that plugins are available to core outside of the node-scope. From one side we want plugins to act like npm-packages, but from the other side we don’t want to put them in the package.json since then we coupled the data that exists in the gem-files.
   We talked about a solution when we search for plugins that have package.json in their root, and then we can run npm install <plugin-location> (without --save), so their code will exist in the core node_modules folder.
   Because plugins won’t consume core (they will consume sub-packages that core uses as well), it makes sense now that core should consume the plugins and we won’t fall into a situation when they depend on each other.

Thanks!

---

Tomer Brisker

to me, Ewoud, Eric, Walden, John, Amir, Ohad

Jan 20

Thanks for the update! Great to hear there’s some progress, let’s get this in asap so we have time to handle any issues before 1.22.

Would be good to also update the rest of the community, either in response to the previous post or as a new post on discourse.

---

Eric Helms

to Tomer, me, Ewoud, Walden, John, Amir, Ohad

Jan 21

Avi,

As I learned more about Insights throughout last week, hearing the UI architecture got me to wondering if it would help solve our issues. The general idea for their design is there is a set of “chroming” that is the common shell, e.g. navigation and routing. Outside of the chroming aspect, each application (or plugin in our case) provides their own UI assets relying on common NPM packages similar to what we envision for foremanVendor and foremanLib. One of the differences being that instead of loading a core bundle and vendor JS that provides all of core’s UI pieces, each plugin would provide it’s UI JS and a vendorJS. Thus, each plugin is coupled to and built against it’s respectively built vendorJS and when a plugin page is loaded, that plugin’s JS is retrieved to load up the plugins pages and interactions. This provides greater independence for every plugin given they are using common components but not relying on a centralized set of built assets (e.g. compiled vendor and compiled library JS). If every plugin provides their own vendor and application JS they would be independent, and without any extra loading given the core JS bundles would not be loaded for every page (only on core Foreman pages).

That’s my interpretation of how it works, and how it could work for us giving plugins more independence and reducing build burden. Thoughts (from anyone)?

As Tomer mentioned, we should move this discussion and results back to upstream, but given this point is using Insights as a reference I started within this smaller thread.

---

Ohad Levy

to Eric, Tomer, me, Ewoud, Walden, John, Amir, Ohad

Jan 21

On Mon, Jan 21, 2019 at 4:56 PM Eric Helms <ehelms@redhat.com> wrote:

Avi,

As I learned more about Insights throughout last week, hearing the UI architecture got me to wondering if it would help solve our issues. The general idea for their design is there is a set of “chroming” that is the common shell, e.g. navigation and routing. Outside of the chroming aspect, each application (or plugin in our case) provides their own UI assets relying on common NPM packages similar to what we envision for foremanVendor and foremanLib. One of the differences being that instead of loading a core bundle and vendor JS that provides all of core’s UI pieces, each plugin would provide it’s UI JS and a vendorJS. Thus, each plugin is coupled to and built against it’s respectively built vendorJS and when a plugin page is loaded, that plugin’s JS is retrieved to load up the plugins pages and interactions. This provides greater independence for every plugin given they are using common components but not relying on a centralized set of built assets (e.g. compiled vendor and compiled library JS). If every plugin provides their own vendor and application JS they would be independent, and without any extra loading given the core JS bundles would not be loaded for every page (only on core Foreman pages).

That’s my interpretation of how it works, and how it could work for us giving plugins more independence and reducing build burden. Thoughts (from anyone)?

I think this is not that different approach from what we would like to end up at, my main question question would be around how to ensure we have a patternfly like place where everyone contribute reusable code to?

based on your explanation above, how is it that different from Avi’s plan? in my head:

1. foreman vendor is all of the common plugins client can use (e.g. common enough that it makes sense to include them once - e.g. react).

2. foreman bundle is the chroming, e.g. navigation and reusable components (e.g. charts, tables, api call logic, redux etc etc)

then each plugin is basically adding its own set of 1 (plugin specific libs) and 2 ( client specific behavior) ?

---

Eric Helms

to Ohad, Tomer, me, Ewoud, Walden, John, Amir

Jan 21

On Mon, Jan 21, 2019 at 10:08 AM Ohad Levy <olevy@redhat.com> wrote:

On Mon, Jan 21, 2019 at 4:56 PM Eric Helms <ehelms@redhat.com> wrote:

Avi,

As I learned more about Insights throughout last week, hearing the UI architecture got me to wondering if it would help solve our issues. The general idea for their design is there is a set of “chroming” that is the common shell, e.g. navigation and routing. Outside of the chroming aspect, each application (or plugin in our case) provides their own UI assets relying on common NPM packages similar to what we envision for foremanVendor and foremanLib. One of the differences being that instead of loading a core bundle and vendor JS that provides all of core’s UI pieces, each plugin would provide it’s UI JS and a vendorJS. Thus, each plugin is coupled to and built against it’s respectively built vendorJS and when a plugin page is loaded, that plugin’s JS is retrieved to load up the plugins pages and interactions. This provides greater independence for every plugin given they are using common components but not relying on a centralized set of built assets (e.g. compiled vendor and compiled library JS). If every plugin provides their own vendor and application JS they would be independent, and without any extra loading given the core JS bundles would not be loaded for every page (only on core Foreman pages).

That’s my interpretation of how it works, and how it could work for us giving plugins more independence and reducing build burden. Thoughts (from anyone)?

I think this is not that different approach from what we would like to end up at, my main question question would be around how to ensure we have a patternfly like place where everyone contribute reusable code to?

based on your explanation above, how is it that different from Avi’s plan? in my head:

1. foreman vendor is all of the common plugins client can use (e.g. common enough that it makes sense to include them once - e.g. react).

2. foreman bundle is the chroming, e.g. navigation and reusable components (e.g. charts, tables, api call logic, redux etc etc)

then each plugin is basically adding its own set of 1 (plugin specific libs) and 2 ( client specific behavior) ?

Exactly. That aspect of the plan does not change. I am looking at changing how we build the assets and then how we serve the assets. Today, we load a core set of JS bundles on every page load that contain both common components and Foreman core application code (e.g. page specific logic that is not re-usable). Those core set of compiled bundles are what plugins require, and build against, in order to run that has created this fragile dependency especially when plugin provided third party libraries are involved. What I am trying to suggest is a model where only code related to the navigation is loaded by default, and depending on the page being accessed determines what other JS bundles are loaded (e.g. core Foreman page logic + Foreman core vendor, Katello page logic + Katello’s vendor). I can try to draw a picture if that would help.

I’ll try to build out a slight example. Say we now have foremanLib with all common components, both foreman-bundle.js and katello-bundle.js would contain copies of foremanLIb with each respective set of code, foremanCore.js and katello.js linked against their respective bundles independently from one another. When on a Foreman core page, katello-bundle.js would not be loaded. On a Katello page, foreman-bundle.js would not be loaded either. Similarly, each wold have their own vendor.js that gets loaded depending on context, but not both together.

---

me

to Eric, Ohad, Tomer, Ewoud, Walden, John, Amir

Jan 23

Eric I love your suggestion and ideally, this how I would want to see it implemented.

You are basically describing SPA architecture when the routing and the navigation done in the client-side while it dynamically imports the relevant JS files needed for the current route.

Then we can use tree-shaking and let webpack create bundles and vendors for core and plugins. As you said we will end up with bigger bundles/vendors but they will be independent.

One of the issues I see is the fact pages are rendered by the server so when you want to use a react component you need to inject a javascript that take the component from a common registry and then mount it to the dom.

While I think it’s a good goal I believe we should stick to the current plan and migrate to an SPA approach when we will have more pages written with React.

---

Tomer Brisker

to me, Eric, Ohad, Ewoud, Walden, John, Amir

Jan 23

Let’s take the discussion to discourse so the rest of the community is aware of it, and most importantly lets start getting the code to solve this written and merged as soon as possible so we can get 1.22 working with webpack as early as possible before branching and have the time to fix any additional issues in either building, packaging, or elsewhere.

[sharvit]

Just created a PR in foreman and in katello so you can play and experiment with it:

WIP: @foreman/vendor sub-package

WIP: @foreman/vendor sub-package

I am using @John_Mitsch webpack-breaking-change-plugin to track diffs between builds.

# create a tmp dir
mkdir public/tmp
# build webpack
bundle exec rake webpack:compile
# save the records.json so we can compare it to a newer records.json later
cp public/webpack/records.json public/tmp/records-v1.json

# rebuild webpack
rm -rf public/webpack
bundle exec rake webpack:compile
cp public/webpack/records.json public/tmp/records-v2.json

# compare them
diff public/tmp/records-v1.json public/tmp/records-v2.json
# At this comparison we should not have any difference
# I do see differences, mostly/only scss

# Rebuild and compare with a minor change
rm -rf public/webpack
# open the webpack/@foreman/vendor/package.json
# update a package z-version
# before: "react-diff-view": "1.8.1",
# after: "react-diff-view": "1.8.0",
# Update the package.json z-version
# before:   "version": "0.0.1",
# after:   "version": "0.0.2",
# clean node_modules
rm -rf node_modules/ package-lock.json webpack/@foreman/vendor/node_modules/ webpack/@foreman/vendor/package-lock.json ../katello/node_modules/ ../katello/package-lock.json
npm i
# build webpack
bundle exec rake webpack:compile
cp public/webpack/records.json public/tmp/records-v3.json
# compare with the old records.json
# there should not be any breaking-changes because we only updates z-version
diff public/tmp/records-v1.json public/tmp/records-v3.json

# Repeat with a y-version update
rm -rf public/webpack
# open the webpack/@foreman/vendor/package.json
# update a package y-version
# before: "react-redux": "5.1.1",
# after: "react-redux": "5.0.0",
# Update the package.json y-version
# before:   "version": "0.0.2",
# after:   "version": "0.1.0",
# clean node_modules
rm -rf node_modules/ package-lock.json webpack/@foreman/vendor/node_modules/ webpack/@foreman/vendor/package-lock.json ../katello/node_modules/ ../katello/package-lock.json
npm i
# build webpack
bundle exec rake webpack:compile
cp public/webpack/records.json public/tmp/records-v4.json
# compare with the old records.json
# Now we should see hash differences with breaking-changes
diff public/tmp/records-v1.json public/tmp/records-v4.json

[sharvit]

To make it easier for you, I uploaded the records.json files:
records.zip.gz (139.9 KB)

[John_Mitsch]

@sharvit PRs look good! One thing to note is you can get the records.json file by adding this line to the webpack config. No need for the whole plugin that diffs the version number and package info, its sounds like we won’t got that direction with the meta package.

[Bernhard_Suttner]

Thanks a lot for this discussions. The big drawback we currently face is, that a plugin P build with foreman build xyz1 requires foreman build xyz1 and “may” not work with xyz2 (= and is therefore even not installable because of the foreman-weback-vendor-js-HASH which is provided by foreman). Means, with every change of foreman it MAY make sense to rebuild ALL plugins - definitely when the HASH has changed. This isn’t very nice and I guess we should think about a solution or better workaround for this.

ideas:

• package the react components / JS / webpack stuff separately in a own package.
• “copy” the required react components / JS / webpack stuff to run a plugin into the plugin rpm package which would result that the plugin itself contains “everything” to be useable.

I know, not the best ideas but the topic is “hard” if you don’t want to rebuild plugins on every change or if you don’t want to risk that a plugin doesn’t work because the webpack dependency is not fulfilled.

[sharvit]

NEW UPDATES: changing direction

We managed to make this RFC works but we feel it adds complexity to the current solution instead simplifying the process.

The new approach we tried includes the new @theforeman/vendor with the following changes:

@theforeman/vendor

1. @theforeman/vendor will build itself using webpack
   1. It will produce dist/foreman-vendor.bundle.js[.gzip] and dist/foreman-vendor.bundle.css[.gzip]
   2. It will use the window object to share modules with core and plugins (example: window.__FOREMAN_VENDOR_REACT__)
2. @theforeman/vendor will have its own lifecycle (build, versioning, deploy to npm/rpm with the dist files)

@theforeman/vendor source
@theforeman/vendor in npm (version: 0.1.0-beta.1)

Foreman build

1. Will use @theforeman/vendor dependency from npm/rpm/source
2. Will use webpack.config where externals are @theforeman/vendor/webpack.externals
   This will ensure webpack will replace those node_modules with the window object.
3. Make sure we serve foreman-vendor.bundle.[js,css]

Foreman branch that uses the new vendor
Diff with develop

• To make it work you will need to build the dist and copy it to the public folder, I am working to fix it.

@ekohl Is currently testing it and working on the packaging side of it and will update soon.

[ehelms]

Thanks for the update Avi!

window.__FOREMAN_VENDOR_REACT__ )

Could you explain a bit more in-depth and provide an example of how this aspect plays out for plugins?

[sharvit]

Could you explain a bit more in-depth and provide an example of how this aspect plays out for plugins?

Sure, webpack has a particular configuration call externals

The externals configuration option provides a way of excluding dependencies from the output bundles. Instead, the created bundle relies on that dependency to be present in the consumer’s environment. This feature is typically most useful to library developers , however there are a variety of applications for it.

So for example, if I would add this to my webpack-config:

{
  externals: {
    react: '__FOREMAN_VENDOR_REACT__'
  }
}

webpack would exclude react from the bundle and will replace it with window.__FOREMAN_VENDOR_REACT__ assuming react exist there.
To make it work, we will need to load a js file before the bundle (vendor) that put react in window.__FOREMAN_VENDOR_REACT__ so react would be available for core and plugins.
e.g., window.__FOREMAN_VENDOR_REACT__ = require('react');

I am using a small webpack-plugin I created to put all the webpack-vendors inside the dist file.
It will result this into the dist file:

window["__FOREMAN_VENDOR__REACT__"] = __webpack_require__(/*! react */ "./node_modules/react/index.js");