gregnb / mui-datatables

Datatables for React using Material-UI
MIT License
2.71k stars 934 forks source link

@material-ui/core import should not be a Top-Level import #1676

Open ghost opened 3 years ago

ghost commented 3 years ago

I'm having an issue with my bundle size, my project is using @material-ui core components as well as mui-datatables. I'm working on shrinking my bundle size, but noticed that the entire esm library for @material-ui/core is getting bundled. I tracked down what is importing the @material-ui/core/esm/index.js module (all the components) and it was mui-datatables. This is happening because the import { Paper, Table as MuiTable, Tooltip as MuiTooltip } from '@material-ui/core'; in mui-datatables/src/MUIDataTable.js is a top-level import and without tree-shaking, the entire MUI component base is pulled into the bundle.

If you think the change is allowable, I can create a PR for it.

Expected Behavior

The @material-ui/core docs say that Library Authors should use path imports like this:

import Paper from '@material-ui/core/Paper';
import MuiTable from '@material-ui/core/Table';
import MuiTooltip from '@material-ui/core/Tooltip';

Current Behavior

Currently, in mui-datatables/src/MUIDataTable.js, the @material-ui/core components are imported from the top level, like this:

import { Paper, Table as MuiTable, Tooltip as MuiTooltip } from '@material-ui/core';

Your Environment

Tech Version
Material-UI 4.11.3
MUI-datatables 3.6.7
React 17.0.1
browser
etc
wdh2100 commented 3 years ago

ref : https://github.com/gregnb/mui-datatables/issues/1603

gooood! 👍

ghost commented 3 years ago

Thanks for making the change! How often are changes made to the main product? Just curious when we can expect to see this in mui-datatables npm registry.

wdh2100 commented 3 years ago

@tylerg-smt

sorry

release 3.7.7

ghost commented 3 years ago

Hello, I believe we should re-open this issue as I found 5 more files importing the Top-Level @material-ui/core. Please see my PR https://github.com/gregnb/mui-datatables/pull/1698

Alternatively I can create a new issue if that is preferable.