CONSOLE-3853: Prevent PatternFly styles from being included in plugin compilation by vojtechszocs · Pull Request #13388 · openshift/console

vojtechszocs · 2023-11-30T18:03:01Z

This PR patches ConsoleRemotePlugin to ensure that PatternFly styles are not included when building a dynamic plugin.

This should reduce the risk of having styling related bugs, i.e. some plugin overriding PatternFly styles provided by Console.

Reference:

How to verify changes in this PR

Switch to latest master branch:

git checkout master
git fetch upstream && git rebase upstream/master # upstream => https://github.com/openshift/console.git

Rebuild Console dynamic plugin SDK and Console demo plugin (dev build for more readable output):

(cd frontend ; yarn) # rebuild frontend/packages/console-dynamic-plugin-sdk/dist
(cd dynamic-demo-plugin ; rm -rf node_modules ; yarn ; yarn build-dev) # rebuild dynamic-demo-plugin/dist

Keep the original Console demo plugin output:

(cd dynamic-demo-plugin ; mv dist dist-before)

Apply this PR on top of your local branch and then repeat step 2. above.
Keep the modified Console demo plugin output:

(cd dynamic-demo-plugin ; mv dist dist-after)

You can now use a visual diff tool like Meld to compare before vs. after build output:

there will be additional chunks containing ignored (empty) PatternFly CSS modules
vendors-node_modules_patternfly_react-core_dist_esm_index_js-chunk.js will refer to these empty CSS modules, for example:

-__webpack_require__(/*! ./alert.css */ "../node_modules/@patternfly/react-styles/css/components/Alert/alert.css");
+__webpack_require__(/*! ./alert.css */ "?9b90");

openshift-ci-robot · 2023-11-30T18:03:05Z

@vojtechszocs: This pull request references CONSOLE-3853 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.15.0" version, but no target version was set.

Details

In response to this:

This PR patches ConsoleRemotePlugin to ensure that PatternFly styles are not included when building a dynamic plugin.

This should reduce the risk of having styling related bugs, i.e. some plugin overriding PatternFly styles provided by Console.

Reference:

webpack resolve.alias option

frontend-components searchIgnoredStyles function

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

vojtechszocs · 2023-11-30T18:04:05Z

frontend/packages/console-dynamic-plugin-sdk/scripts/package-definitions.ts

      ...parseDeps(
        rootPackage,
-        ['ajv', 'chalk', 'comment-json', 'find-up', 'read-pkg', 'semver'],
+        ['ajv', 'chalk', 'comment-json', 'find-up', 'glob', 'read-pkg', 'semver'],


This essentially adds glob to @openshift-console/dynamic-plugin-sdk-webpack package dependencies.

vojtechszocs · 2023-11-30T18:05:16Z

frontend/packages/console-dynamic-plugin-sdk/src/webpack/ConsoleRemotePlugin.ts

    compiler.options.output.publicPath = publicPath;

+    compiler.options.resolve = compiler.options.resolve ?? {};
+    compiler.options.resolve.alias = compiler.options.resolve.alias ?? {};


FYI, can't use the ??= operator here due to an older Prettier version.

vojtechszocs · 2023-11-30T18:06:41Z

frontend/packages/console-dynamic-plugin-sdk/src/webpack/ConsoleRemotePlugin.ts

+    // Prevent PatternFly styles from being included in the compilation
+    getPatternFlyStyles().forEach((cssFile) => {
+      if (Array.isArray(compiler.options.resolve.alias)) {
+        compiler.options.resolve.alias.push({ name: cssFile, alias: false });


options.resolve.alias is typically an object, but webpack also allows for an array representation.

openshift-ci-robot · 2023-11-30T18:43:38Z

@vojtechszocs: This pull request references CONSOLE-3853 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.15.0" version, but no target version was set.

Details

In response to this:

This PR patches ConsoleRemotePlugin to ensure that PatternFly styles are not included when building a dynamic plugin.

This should reduce the risk of having styling related bugs, i.e. some plugin overriding PatternFly styles provided by Console.

Reference:

webpack resolve.alias option

frontend-components searchIgnoredStyles function

How to verify changes in this PR:

Switch to latest master branch:
git checkout master
git fetch upstream && git rebase upstream/master # upstream => https://github.com/openshift/console.git
Rebuild Console dynamic plugin SDK and Console demo plugin:
(cd frontend ; yarn) # rebuild frontend/packages/console-dynamic-plugin-sdk/dist
(cd dynamic-demo-plugin ; rm -rf node_modules ; yarn ; yarn build-dev) # rebuild dynamic-demo-plugin/dist
Keep the original Console demo plugin output:
(cd dynamic-demo-plugin ; mv dist dist-before)
Apply this PR on top of your local branch and then repeat step 2. above.

Keep the modified Console demo plugin output:
(cd dynamic-demo-plugin ; mv dist dist-after)
You can now use a visual diff tool like Meld to compare before vs. after build output:

there will be additional chunks containing ignored (empty) PatternFly CSS modules

vendors-node_modules_patternfly_react-core_dist_esm_index_js-chunk.js will refer to these empty CSS modules, for example:
-__webpack_require__(/*! ./alert.css */ "../node_modules/@patternfly/react-styles/css/components/Alert/alert.css");
+__webpack_require__(/*! ./alert.css */ "?9b90");

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot · 2023-11-30T18:46:48Z

@vojtechszocs: This pull request references CONSOLE-3853 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.15.0" version, but no target version was set.

Details

In response to this:

This PR patches ConsoleRemotePlugin to ensure that PatternFly styles are not included when building a dynamic plugin.

This should reduce the risk of having styling related bugs, i.e. some plugin overriding PatternFly styles provided by Console.

Reference:

webpack resolve.alias option

frontend-components searchIgnoredStyles function

How to verify changes in this PR:

Switch to latest master branch:
git checkout master
git fetch upstream && git rebase upstream/master # upstream => https://github.com/openshift/console.git
Rebuild Console dynamic plugin SDK and Console demo plugin (dev build for more readable output):
(cd frontend ; yarn) # rebuild frontend/packages/console-dynamic-plugin-sdk/dist
(cd dynamic-demo-plugin ; rm -rf node_modules ; yarn ; yarn build-dev) # rebuild dynamic-demo-plugin/dist
Keep the original Console demo plugin output:
(cd dynamic-demo-plugin ; mv dist dist-before)
Apply this PR on top of your local branch and then repeat step 2. above.

Keep the modified Console demo plugin output:
(cd dynamic-demo-plugin ; mv dist dist-after)
You can now use a visual diff tool like Meld to compare before vs. after build output:

there will be additional chunks containing ignored (empty) PatternFly CSS modules

vendors-node_modules_patternfly_react-core_dist_esm_index_js-chunk.js will refer to these empty CSS modules, for example:
-__webpack_require__(/*! ./alert.css */ "../node_modules/@patternfly/react-styles/css/components/Alert/alert.css");
+__webpack_require__(/*! ./alert.css */ "?9b90");

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot · 2023-11-30T18:52:01Z

@vojtechszocs: This pull request references CONSOLE-3853 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.15.0" version, but no target version was set.

Details

In response to this:

This PR patches ConsoleRemotePlugin to ensure that PatternFly styles are not included when building a dynamic plugin.

This should reduce the risk of having styling related bugs, i.e. some plugin overriding PatternFly styles provided by Console.

Reference:

webpack resolve.alias option

frontend-components searchIgnoredStyles function

How to verify changes in this PR

Switch to latest master branch:
git checkout master
git fetch upstream && git rebase upstream/master # upstream => https://github.com/openshift/console.git
Rebuild Console dynamic plugin SDK and Console demo plugin (dev build for more readable output):
(cd frontend ; yarn) # rebuild frontend/packages/console-dynamic-plugin-sdk/dist
(cd dynamic-demo-plugin ; rm -rf node_modules ; yarn ; yarn build-dev) # rebuild dynamic-demo-plugin/dist
Keep the original Console demo plugin output:
(cd dynamic-demo-plugin ; mv dist dist-before)
Apply this PR on top of your local branch and then repeat step 2. above.

Keep the modified Console demo plugin output:
(cd dynamic-demo-plugin ; mv dist dist-after)
You can now use a visual diff tool like Meld to compare before vs. after build output:

there will be additional chunks containing ignored (empty) PatternFly CSS modules

vendors-node_modules_patternfly_react-core_dist_esm_index_js-chunk.js will refer to these empty CSS modules, for example:
-__webpack_require__(/*! ./alert.css */ "../node_modules/@patternfly/react-styles/css/components/Alert/alert.css");
+__webpack_require__(/*! ./alert.css */ "?9b90");

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

vojtechszocs · 2023-12-01T15:27:10Z

/test e2e-gcp-console

vojtechszocs · 2023-12-06T15:28:28Z

Running: knative-ci.feature (1 of 1)

...

<--- JS stacktrace --->

[16777:1201/191957.619205:ERROR:v8_initializer.cc(730)] V8 process OOM (Oilpan: Reserving memory.).

We detected that the Chromium Renderer process just crashed.

/retest

TheRealJon

/lgtm

openshift-ci · 2023-12-07T21:55:48Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: TheRealJon, vojtechszocs

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~dynamic-demo-plugin/OWNERS~~ [TheRealJon,vojtechszocs]
~~frontend/OWNERS~~ [TheRealJon,vojtechszocs]

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

jhadvig · 2023-12-08T10:34:44Z

QE Approver:
/assign @yanpzhan
Docs Approver: 
/assign @opayne1
PX Approver:
/assign @RickJWagner

opayne1 · 2023-12-08T12:03:49Z

/label docs-approved

yanpzhan · 2023-12-08T12:29:08Z

Checked the code according to @vojtechszocs 's detailed steps, compared files in dist-before and dist-after,
there are additional chunks containing ignored (empty) PatternFly CSS modules:

_a268-_9d82-_21bc-_d0d2-_b014-_9b90-_2347-_271e-_1cde-_eaa9-_6e60-_1574-_ef4e-_fa93-_beef-_a1-113a3b0-chunk.js
_a268-_9d82-_21bc-_d0d2-_b014-_9b90-_2347-_271e-_1cde-_eaa9-_6e60-_1574-_ef4e-_fa93-_beef-_a1-113a3b1-chunk.js

And also could see the difference in vendors-node_modules_patternfly_react-core_dist_esm_index_js-chunk.js :

-__webpack_require__(/*! ./alert.css */ "../node_modules/@patternfly/react-styles/css/components/Alert/alert.css");
+__webpack_require__(/*! ./alert.css */ "?9b90");

/label qe-approved

openshift-ci-robot · 2023-12-08T12:29:13Z

@vojtechszocs: This pull request references CONSOLE-3853 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.16.0" version, but no target version was set.

Details

In response to this:

This PR patches ConsoleRemotePlugin to ensure that PatternFly styles are not included when building a dynamic plugin.

This should reduce the risk of having styling related bugs, i.e. some plugin overriding PatternFly styles provided by Console.

Reference:

webpack resolve.alias option

frontend-components searchIgnoredStyles function

How to verify changes in this PR

Switch to latest master branch:
git checkout master
git fetch upstream && git rebase upstream/master # upstream => https://github.com/openshift/console.git
Rebuild Console dynamic plugin SDK and Console demo plugin (dev build for more readable output):
(cd frontend ; yarn) # rebuild frontend/packages/console-dynamic-plugin-sdk/dist
(cd dynamic-demo-plugin ; rm -rf node_modules ; yarn ; yarn build-dev) # rebuild dynamic-demo-plugin/dist
Keep the original Console demo plugin output:
(cd dynamic-demo-plugin ; mv dist dist-before)
Apply this PR on top of your local branch and then repeat step 2. above.

Keep the modified Console demo plugin output:
(cd dynamic-demo-plugin ; mv dist dist-after)
You can now use a visual diff tool like Meld to compare before vs. after build output:

there will be additional chunks containing ignored (empty) PatternFly CSS modules

vendors-node_modules_patternfly_react-core_dist_esm_index_js-chunk.js will refer to these empty CSS modules, for example:
-__webpack_require__(/*! ./alert.css */ "../node_modules/@patternfly/react-styles/css/components/Alert/alert.css");
+__webpack_require__(/*! ./alert.css */ "?9b90");

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

RickJWagner · 2023-12-08T13:13:42Z

/label px-approved

openshift-ci-robot · 2023-12-08T16:06:09Z

/retest-required

Remaining retests: 0 against base HEAD b6b1302 and 2 for PR HEAD 848987c in total

openshift-ci-robot · 2023-12-08T22:34:36Z

/retest-required

Remaining retests: 0 against base HEAD 468b9b4 and 1 for PR HEAD 848987c in total

openshift-ci · 2023-12-09T11:07:31Z

@vojtechszocs: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

openshift-bot · 2023-12-09T15:00:56Z

[ART PR BUILD NOTIFIER]

This PR has been included in build openshift-enterprise-console-container-v4.16.0-202312091410.p0.gad807f6.assembly.stream for distgit openshift-enterprise-console.
All builds following this will include this PR.

Prevent PatternFly styles from being included in plugin compilation

848987c

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Nov 30, 2023

vojtechszocs commented Nov 30, 2023

View reviewed changes

openshift-ci bot requested review from kyoto and rawagner November 30, 2023 18:06

openshift-ci bot added component/sdk Related to console-plugin-sdk approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Nov 30, 2023

vojtechszocs commented Nov 30, 2023

View reviewed changes

TheRealJon approved these changes Dec 7, 2023

View reviewed changes

openshift-ci bot assigned TheRealJon Dec 7, 2023

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 7, 2023

openshift-ci bot added the docs-approved Signifies that Docs has signed off on this PR label Dec 8, 2023

openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Dec 8, 2023

openshift-ci bot added the px-approved Signifies that Product Support has signed off on this PR label Dec 8, 2023

openshift-merge-bot bot merged commit ad807f6 into openshift:master Dec 9, 2023

vojtechszocs mentioned this pull request Feb 1, 2024

OCPBUGS-28917: Optimize federation of shared PF5 code for dynamic plugins #13566

Merged

sg00dwin mentioned this pull request Sep 6, 2024

OCPBUGS-38845: Update vendor imports to include all PatternFly components #14246

Merged

logonoff deleted the improve-pf-mod-fed branch September 25, 2025 13:42

Conversation

vojtechszocs commented Nov 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

How to verify changes in this PR

Uh oh!

openshift-ci-robot commented Nov 30, 2023 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vojtechszocs Nov 30, 2023

Choose a reason for hiding this comment

Uh oh!

vojtechszocs Nov 30, 2023

Choose a reason for hiding this comment

Uh oh!

vojtechszocs Nov 30, 2023

Choose a reason for hiding this comment

Uh oh!

openshift-ci-robot commented Nov 30, 2023 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented Nov 30, 2023 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented Nov 30, 2023 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

How to verify changes in this PR

Uh oh!

vojtechszocs commented Dec 1, 2023

Uh oh!

vojtechszocs commented Dec 6, 2023

Uh oh!

TheRealJon left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci bot commented Dec 7, 2023

Uh oh!

jhadvig commented Dec 8, 2023

Uh oh!

opayne1 commented Dec 8, 2023

Uh oh!

yanpzhan commented Dec 8, 2023

Uh oh!

openshift-ci-robot commented Dec 8, 2023 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

How to verify changes in this PR

Uh oh!

RickJWagner commented Dec 8, 2023

Uh oh!

openshift-ci-robot commented Dec 8, 2023

Uh oh!

openshift-ci-robot commented Dec 8, 2023

Uh oh!

openshift-ci bot commented Dec 9, 2023

Uh oh!

openshift-bot commented Dec 9, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

vojtechszocs commented Nov 30, 2023 •

edited

Loading

openshift-ci-robot commented Nov 30, 2023 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Nov 30, 2023 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Nov 30, 2023 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Nov 30, 2023 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Dec 8, 2023 •

edited by openshift-ci bot

Loading