Bump PatternFly to latest

[rhamilto]

https://jira.coreos.com/browse/CONSOLE-1447

[spadgett]

cc @dtaylor113 @rebeccaalpert

[spadgett]

@priley86 I know you needed this in your tables PR... What impact does this have on our builds?

[priley86]

this is required due to react-core change in import methodology. CSS will be loaded directly from node_modules/@patternfly from our JS requires.

I used the quick and easy method in the tables PR, but we should probably be more selective so that directories unexpected are not scanned for CSS. Looks we can do something like:

      {
        test: /\.s?css$/,
        exclude: /node_modules\/(?!(@patternfly)\/).*/,
      }

or

      {
        test: /\.s?css$/,
        exclude: function(modulePath) {
          return /node_modules/.test(modulePath) &&
              !/node_modules\/@patternfly/.test(modulePath);
        },
      }

Webpack does not seem to respect:

      {
        test: /\.s?css$/,
        exclude: /node_modules/,
        include: [/node_modules\/@patternfly/],
      }

source: webpack/webpack#2031 (comment)

cc: @dgutride

[spadgett]

@priley86 It looks like this change pulls in a bunch of CSS we don't use and increases our total CSS by 50%

[dgutride]

@spadgett - I'd agree with that assessment and this is a perfect use case for our more advanced PatternFly css architecture. For OpenShift, my recommendation would be to investigate use of a null loader (https://webpack.js.org/loaders/null-loader/) for the css require statements that have been introduced by react-core. Since your team is importing the full patternfly css from @ patternfly/patternfly - component specific css can be discarded.

[spadgett]

@dgutride we're not importing the full patternfly css... unfortunately null loader just results in a bunch of missing styles :/

[dgutride]

The end result is that you have the same amount of CSS but it is managed far more efficiently and you don't have to worry about garbage collection between routes or DOM bloat over time (and caching of css files leads to better efficiency).

[rhamilto]

With @priley86's method, we're still seeing the inclusion of CSS for components we're not using. Is this intended?

[rhamilto]

still seeing the inclusion of CSS for components we're not using

It in fact appears to be all components and utilities. This does not seem desirable.

[rhamilto]

@priley86 confirmed all of PF4 CSS is pulled in only during dev as treeshaking occurs for prod and only relevant CSS is bundled.

The increase in the dist vendor bundle size is expected as the majority of PF4 CSS is no longer being injected in to head via emotion, but instead is being bundled.

@spadgett, this PR is ready for final review.

[priley86]

please inspect webpack production build when checking this pf4 upgrade. there may be additional reductions we can add later.

[rhamilto]

/retest

[rhamilto]

/retest

[spadgett]

/lgtm

Let's go ahead with this change. We can continue looking at webpack builds as follow-on work.

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rhamilto, spadgett

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• frontend/OWNERS [rhamilto,spadgett]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment