patternfly / patternfly-react

A set of React components for the PatternFly project.
https://react-staging.patternfly.org/
MIT License
798 stars 355 forks source link

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

Open jelly opened 5 months ago

jelly commented 5 months ago

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.

martinpitt commented 5 months ago

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";
jelly commented 5 months ago

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

github-actions[bot] commented 1 month ago

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

jelly commented 1 month ago

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