RFC: Changing how we handle Webpack Building

Development RFCs
53

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.

54

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

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
1 Like
55

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

56

@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.

57

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.

58

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.

1 Like
59

Thanks for the update Avi!

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

60

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");
2 Likes
61

Updates with @theforeman/vendor@0.1.0-beta.5:

  1. npm run build will produce files with hashes in their names
  2. npm run build will produce a dist/manifest.json with mapping between the chunk name to the files.
  3. Created easier integration with a webpack-plugin
    1. The plugin will copy the dist files the consumer outputPath (e.g. public/webpack)
    2. The plugin will update the consumer manifest.json to include the dist files there (will work with dev-server and production)
    3. The plugin will update the consumer externals configuration.

You can try it with this branch (not have to change plugins to make it work): https://github.com/theforeman/foreman/compare/develop...sharvit:feature/foreman-vendor

1 Like
62

Who releases this and when? Would it make sense to have this live in the foreman git repository?

Currently I’m attempting to do a PoC packaging change for this. I noticed this generates a 34M gzipped cache file which is unacceptable. However, it looks like the current npm2rpm includes all sources into its cache while also fetch the sources from npmjs.org. That’s a bug we need to get rid of before this is merged. With that solved, it should be much smaller again.

63

Can you elaborate on why this unacceptable?

64

Will this mapping be provided by Foreman core or something each plugin has to implement based on their library usage?

What is the strategy for plugins that have dependencies not in foreman-vendor?

65

I’m guessing 99% of that is the actual NPM package sources which we already have separated out. The cache is intended to only cache the JSON files. Those are only a fraction of that size. I’m probably the one to blame for that regression in npm2rpm because I’m pretty sure it wasn’t always like that.

66

Status update on the packaging side. I have a draft PR with a bundle file that builds locally. Haven’t tried to build the Foreman RPM with the right sources yet.

https://github.com/theforeman/foreman-packaging/pull/3525

It does make clear how much duplication there is. Better than before. For example, it’s clear there’s a react-bootstrap 0.32.1 and 0.32.4 included. That doesn’t sounds right to me.

In the past I did patch around this in a few places and sent patches upstream. It’d be good if more people took the effort. It has many benefits, like smaller npm installs and fewer bugs.

1 Like
67

There are many options, my suggestion:

Have a separate git repository theforeman/foreman-frontend where we can put multiple sub-packages.
atm we can put there @theforeman/vendor and @theforeman/test-utils from https://github.com/sharvit/react-redux-test-utils (we had a discussion we want it under theforeman org)
And I believe we have a case for more sub-packages to create in the future.
patternfly-react do the same while they are managing multiple projects in one repo using lerna.
With lerna we can have separate lifecycles and automated releases for each sub-package.

The reason I don’t want it to be apart of core because it will lead to lots of confusions about how staff links together when they live together in the same repo but got consumed from npm.
If you will not use npm and you will link them locally, you will lose the versioning benefit of the sub-packages, then you have the issue of how plugins should consume those packages.

atm I don’t think we should get that forward with automations, I think we should tag releases manually and let Travis deploy npm and rpm (I don’t know much about rpms) when we create a new release commit and tag manually after X prs/days.
right now my repo use travis to deploy to npm when I create a new git tag and update the package.json.

68

Because atm core build the plugins, the plugins don’t need to change anything.
But core don’t really need to implement anything, @theforeman/vendor provides a webpack-package that do the mapping for you. All you need to do is adding this plugin to you webpack config:

// webpack.config.js
const ForemanVendorPlugin = require('@theforeman/vendor/webpack.plugin');

module.exports = {
  entry: { ... },
  output: { ... },
  module: { ... },
  plugins: [
    new ForemanVendorPlugin(),
    ...
  ],
};

Once plugins would be able to build themselves (and we should aim there), consuming @theforeman/vendor mapping would be easy as that.
Those mapping will provide to core and plugins all the mappings included in https://github.com/sharvit/foreman-vendor/blob/master/lib/webpack-vendor.js

While this is the only change you will need to do to run dev-server and build prod/dev, there are other changes you will need to do in your project as a consumer of @theforeman/vendor so you can run tests, storybook and linting and those changes described in the readme file:
https://github.com/sharvit/foreman-vendor

69

If plugin (or core) depend on a package include in foreman-vendor it will ignore the local package and will use the package included in foreman-vendor.
If a package not included in foreman-vendor it will compile this package into the core/plugin bundle file.

70

We might be able to reduce it, you need different packages for different use-cases.

Build the foreman-vendor dist files
Needs all the packages, once done, you have dist files in npm.

Run foreman dev-server, lint, storybook, testing
Needs all the nonDev dependencies in foreman-vendor package.json

Build foreman/plugins production/dev bundles
Needs only one package from the foreman-vendor package.json, copy-webpack-plugin.

Can we reduce build sizes around it?

71

To be perfectly clear: currently we have all sources (babel-x.y.z.tgz, react-x.y.z.tgz) listed in the spec file. We also have a tarball that contains the cached responses from npm (PACKAGE-registry-x.y.z.tgz) which we check into git directly. It turns out that also contains the sources (babel-x.y.z.tgz, …). This duplication doesn’t serve any purpose and only blows up our git repository size. I think a patch that clears the duplication and reuses the sources should be easy to come up with.

72

A quick test removing the actual sources reduced the cache size from 34M to 14M. Will proceed with that further. Looks like we’ve been duplicating that for quite some time but this ridiculous size made it obvious.

I do think it’d be good to have a package that you can add to your devRequires and one to your requires. From the packaging side that would greatly improve our life.

2 Likes