patternfly-react icon indicating copy to clipboard operation
patternfly-react copied to clipboard

Bug - [react-table] - Using a Table component causes all of PatternFly's CSS to be imported

Open jelly opened this issue 1 year ago • 4 comments

Describe the problem

We have been using esbuild for a while now with Cockpit, and recently we found that it includes a bundle analyzer. Our bundle is great large (uncompressed) so it would be interesting to see if we can make it smaller.

(If you want to see this in action, upload this meta.json file to the esbuild bundle analyzer).

What stands out is the 1.5 mb of react-styles which are imported, this project (cockpit-machines) does not use a Masthead component but the css is still imported.

image

The bundle analyzer can also show why something is imported

image

So importing Table/index.js imports

import { useOUIAProps, handleArrows, setTabIndex } from '@patternfly/react-core';

The problem here is that @patternfly/react-core is a barrel file, so importing it this way imports @patternfly/react-core/dist/js/index.js:

[jelle@t14s][~/projects/cockpit-machines]%cat -p node_modules/@patternfly/react-core/dist/esm/index.js
export * from './components';
export * from './layouts';
export * from './helpers';
export * from './styles';
//# sourceMappingURL=index.js.map

So basically it imports all of PatternFly, esbuild seems to be smart enough to not include unused JavaScript but it seems it doesn't handle CSS.

An easy fix would be for react-table to import for example setTabIndex from react-core/dist/js/helpers/KeyboardHandler.js directly.

jelly avatar Jun 17 '24 07:06 jelly

In fact we do this a lot in our projects: We never import the full @patternfly/react-core', but only the components which we need, like in https://github.com/cockpit-project/cockpit-machines/blob/main/src/components/networks/network.jsx:

import { Button } from "@patternfly/react-core/dist/esm/components/Button";
import { DropdownItem } from "@patternfly/react-core/dist/esm/components/Dropdown";
import { Tooltip } from "@patternfly/react-core/dist/esm/components/Tooltip";

martinpitt avatar Jun 17 '24 07:06 martinpitt

I looked at react-table and edited our node_modules to see if it would make an impact:

diff --git a/@patternfly/react-table/dist/esm/components/Table/Table.js b/@patternfly/react-table/dist/esm/components/Table/Table.js
index 702ab38a2d..e6c40d5ace 100644
--- a/@patternfly/react-table/dist/esm/components/Table/Table.js
+++ b/@patternfly/react-table/dist/esm/components/Table/Table.js
@@ -5,7 +5,8 @@ import stylesGrid from '@patternfly/react-styles/css/components/Table/table-grid
 import stylesTreeView from '@patternfly/react-styles/css/components/Table/table-tree-view.mjs';
 import { css } from '@patternfly/react-styles';
 import { toCamel } from './utils';
-import { useOUIAProps, handleArrows, setTabIndex } from '@patternfly/react-core';
+import { handleArrows, setTabIndex } from '@patternfly/react-core/dist/esm/helpers/KeyboardHandler';
+import { useOUIAProps } from "@patternfly/react-core/dist/esm/helpers/OUIA/ouia";
 import { TableGridBreakpoint } from './TableTypes';
 export const TableContext = React.createContext({
     registerSelectableRow: () => { }

Old meta.json New meta.json

In numbers the result is: 8 mb input size => 4.7 mb output size 6.7 mb input size => 4.0 mb output size

This would be a small but nice reduction in final bundle size for cockpit-ostree, for cockpit-machines I would need to patch more PF components to get similar results.

Old: image

New: image

jelly avatar Jun 17 '24 09:06 jelly

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

github-actions[bot] avatar Oct 09 '24 11:10 github-actions[bot]

Need to recheck with PF6 but this is still an issue in PF5.

jelly avatar Oct 09 '24 12:10 jelly