-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[core] Don't export internal utils #2233
[core] Don't export internal utils #2233
Conversation
873a6cf
to
181968a
Compare
@@ -3,7 +3,7 @@ import PropTypes from 'prop-types'; | |||
import clsx from 'clsx'; | |||
import { useFakeTimers } from 'sinon'; | |||
import { withStyles } from '@material-ui/styles'; | |||
import { createTheme } from '@material-ui/data-grid'; | |||
import { createTheme } from '../../packages/grid/_modules_/grid/utils/utils'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting, we can't use @material-ui/data-grid/utils/utils
because of the _modules_
folder structure. This folder structure prevents us to split MIT and Commercially licensed code. So we need to move most of the modules away. Once we do how do XGrid and DataGrid share the utils? I guess we have to either keep the utils in or export them as unstable_
. Anyway, it's a problem for our future self. Making the utils private is the right thing to do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, removing the unnecessary exports is a first step to help us reorganize the codebase between the MIT and the Commercially licensed code.
And there is some code that is not Grid related and should not be exported and not be in a grid
folder but probably in a _shared_
directory that will be accessed by the grids but also the new X components.
That's the case for a lot of the utils.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but probably in a shared directory that will be accessed by the grids but also the new X components.
If it needs to be access cross components, then we need a similar package to @material-ui/utils
. Maybe we should name them:
@mui/core-utils
(prev@material-ui/utils
)@mui/x-utils
Related to https://www.notion.so/mui-org/Rearranging-the-packages-2f700caf8b5e48559fe86ebb648ed91a
+1 for re-implementation |
OK, copied that there have been changes applied. pre Update import { createTheme, DataGrid, GridCellParams, GridColDef, GridEditCellPropsParams, GridRowModel, GridRowSelectedParams, GridSelectionModelChangeParams, GRID_CELL_EDIT_PROPS_CHANGE } from '@material-ui/data-grid'
[...]
const renderEditCell = (params: GridCellParams) => {
return <EditTypeOfProduct {...params} Companys={values.Companys} />
} After the update Module '"@material-ui/data-grid"' has no exported member 'createTheme'.
Module '"@material-ui/data-grid"' has no exported member 'GridRowSelectedParams'.
Module '"@material-ui/data-grid"' has no exported member 'GridSelectionModelChangeParams'. Is there a new alternative way? |
@Rupert-com
import { createTheme } from '@material-ui/core/styles';
|
Part of #2227
The following variables and types are not exported anymore from
@material-ui/data-grid
and@material-ui/x-grid
These three utils are used in the demo.
Do we want to export them or let the user re-implement them ?
For
escapeRegExp
andisOverflown
, I don't think it's the role of MUI to provide such generic methods.For
getThemePaletteMode
, the core could export it but if it does not I suppose we shouldn't do it