Skip to content
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

[RFC] Clean x-data-grid and x-data-grid-pro exports #2227

Closed
flaviendelangle opened this issue Jul 28, 2021 · 9 comments
Closed

[RFC] Clean x-data-grid and x-data-grid-pro exports #2227

flaviendelangle opened this issue Jul 28, 2021 · 9 comments
Assignees
Labels
component: data grid This is the name of the generic UI component, not the React module! new feature New feature or request RFC Request For Comments

Comments

@flaviendelangle
Copy link
Member

flaviendelangle commented Jul 28, 2021

Relates to #924
Relates to #18

Proposals

  1. Stop exporting everything from the grid
export * from './components';
export * from './constants';
export * from './hooks';
export * from './locales';
export * from './models';
export * from './utils';
export * from './GridComponentProps';

We currently export from the module root almost every variable / type ever exported from any file inside the grid codebase.
We should be able to keep some variables / types private

And even if we have an export * cascade, we almost always import from the leafs to avoid circular imports.

Here are a few things we should stop exporting without reason:

  • the features hooks (DataGrid exports useGridColumnReorder even if it does not use it). We may export them when we'll have a hook-only API but currently they can never be properly used outside of the Grid.

  • the internal utilities, (isMultipleKey, isEscapeKey, isDesc, ...)

  • the internal data transformation functions (mergeGridColTypes, ...)

  1. Group the event constants into a single enum

We are exporting dozens of variables (GRID_RESIZE, GRID_KEYDOWN, GRID_COLUMN_HEADER_FOCUS) when we could only export a single enum GridEvent.
This would be more explicit and would also allow to strongly type publishEvent first argument (and later on type the second argument).

  1. Group the CSS class variables

Probably related to #1950

Motivations

  • Limit the breaking changes. If we export everything, renaming an internal variable or type is theoretically a breaking change. In a PR currently in work, I stop exporting DEFAULT_GRID_SLOTS_COMPONENTS because I moved it inside the single file that is using it, it's breaking because the variable is exported even if never used by anyone.

  • Make the import clearer. Having hundreds of imports is a bit overwhelming.

  • Never export something related to XGrid in the DataGrid package

  • Make it easier to reduce bundle size of DataGrid by knowing what is in it

@flaviendelangle flaviendelangle added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jul 28, 2021
@flaviendelangle flaviendelangle self-assigned this Jul 28, 2021
@flaviendelangle flaviendelangle added new feature New feature or request and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Jul 28, 2021
@m4theushw
Copy link
Member

  1. Stop exporting everything from the grid

TypeDoc only generates documentation for what is exported. By not exporting a particular interface we might lose its docs. We have to have this in mind.

the internal utilities, (isMultipleKey, isEscapeKey, isDesc, ...)

These we can definitely stop exporting them ASAP.

the internal data transformation functions (mergeGridColTypes, ...)

Same as above. +1 for not exporting.

  1. Group the event constants into a single enum

I'm not sure if the docs will still be generated correctly. We relay in the @event annotation to know if it's an event. To group events a string enum has to be used to keep the string value. Basically, the constant (e.g. GRID_ROW_CLICK) is for internal use only and, in the markdown and demos, the string (e.g. "rowClick") is encouraged because it's simpler to use. We had this discussion in #1299 (comment)

  1. Group the CSS class variables

Could you elaborate your idea? We're using constants but there're some classes still hardcoded. We can even leverage more the generateUtilityClasses to support our convention.

https://github.com/mui-org/material-ui-x/blob/21fd8f4606fd101ab657092b45eb7a8fdfc25a12/packages/grid/_modules_/grid/components/panel/GridPanel.tsx#L47

@flaviendelangle
Copy link
Member Author

flaviendelangle commented Jul 28, 2021

TypeDoc only generates documentation for what is exported.

We should be able to add a TypeDoc entry point directly on the event file I think.
But I agree, the doc must not be impacted

I'm not sure if the docs will still be generated correctly

I agree that it must still generate the doc correctly and that if it can't them the enum is not viable.
If you think it's better for the user to use the string directly instead of the constant, then maybe stop exporting all the event constants would be enough.

Could you elaborate your idea?

I'm not sure we should touch to the class part since it must be refactored in #1950
But I'm talking about this file https://github.com/mui-org/material-ui-x/blob/master/packages/grid/_modules_/grid/constants/cssClassesConstants.ts
In a general way, I think that if we have more than a few related primitive variables exported, then an object would clean the public API.
Right now to get the class of an element in the IDE, you type CLASS and the autocomplete gives you the constants and the utilities, which make quite a few results. I think it would be easier to use if you just import gridClasses and then you all the values in it are classes.
But that's pretty much what #1950 will do.

@oliviertassinari
Copy link
Member

As a side note, rebasing #1212 allows having a great overview of the expected modules, see https://deploy-preview-1212--material-ui-x.netlify.app/ for @material-ui/data-grid. For instance, It's weird that we export Signature to the public.

@flaviendelangle
Copy link
Member Author

I was using Webstorm autocomplete, but a raw TypeDoc implementation is a lot easier to follow indeed.

@flaviendelangle
Copy link
Member Author

flaviendelangle commented Oct 6, 2021

Element currently exported (I did not list the types for now)
This list does not include the elements removed in #2789 and #2788

// License
- LicenseInfo // x-data-grid-pro only
- GridSignature


// Column constants and methods
- DEFAULT_GRID_COL_TYPE_KEY // TODO: Rename GRID_DEFAULT_COL_TYPE_KEY
- GRID_DEFAULT_LOCALE_TEXT // TODO: Remove from export ?
- GRID_EXPERIMENTAL_ENABLED // TODO: Remove from export ?

- GRID_BOOLEAN_COLUMN_TYPE // Fixed in #2791: Remove from export (not used internally)
- GRID_DATETIME_COLUMN_TYPE // Fixed in #2791: Remove from export (not used internally)
- GRID_DATE_COLUMN_TYPE // Fixed in #2791: Remove from export (not used internally)
- GRID_NUMBER_COLUMN_TYPE // Fixed in #2791: Remove from export + Stop use internally
- GRID_STRING_COLUMN_TYPE // Fixed in #2791: Remove from export (not used internally

- GRID_ACTIONS_COL_DEF
- GRID_DATETIME_COL_DEF
- GRID_DATE_COL_DEF
- GRID_NUMERIC_COL_DEF
- GRID_SINGLE_SELECT_COL_DEF
- GRID_STRING_COL_DEF
- gridCheckboxSelectionColDef // Fixed in #2793: Rename "GRID_CHECKBOX_SELECTION_COL_DEF"


// Fixed in #2793: Export getGridBooleanOperators
- getGridDateOperators
- getGridNumericColumnOperators
- getGridSingleSelectOperators
- getGridStringOperators


// Other constants
- SUBMIT_FILTER_STROKE_TIME // TODO: Remove from export ?


// Components
- DataGridPro (or DataGrid for `x-data-grid`)
- GridActionsCell
- GridActionsCellItem
- GridAddIcon
- GridArrowDownwardIcon
- GridArrowUpwardIcon
- GridAutoSizer
- GridBody
- GridCell
- GridCellCheckboxForwardRef
- GridCellCheckboxRenderer
- GridCheckCircleIcon
- GridCheckIcon
- GridCloseIcon
- GridColumnHeaderItem
- GridColumnHeaderMenu
- GridColumnHeaderSeparator
- GridColumnHeaderSortIcon
- GridColumnHeaderTitle
- GridColumnHeadersItemCollection
- GridColumnIcon
- GridColumnMenu
- GridColumnMenuContainer
- GridColumnsContainer
- GridColumnsHeader
- GridColumnsMenuItem
- GridColumnsPanel
- GridDataContainer
- GridDragIcon
- GridEditInputCell
- GridEditSingleSelectCell
- GridEmptyCell
- GridErrorHandler
- GridFilterAltIcon
- GridFilterForm
- GridFilterInputValue
- GridFilterListIcon
- GridFilterMenuItem
- GridFilterPanel
- GridFooter
- GridFooterContainer
- GridFooterPlaceholder
- GridHeader
- GridHeaderCheckbox
- GridHeaderPlaceholder
- GridLoadIcon
- GridLoadingOverlay
- GridMenu
- GridMenuIcon
- GridMoreVertIcon
- GridNoRowsOverlay
- GridOverlay
- GridOverlays
- GridPagination
- GridPanel
- GridPanelContent
- GridPanelFooter
- GridPanelHeader
- GridPanelWrapper
- GridPreferencesPanel
- GridRenderingZone
- GridRoot
- GridRow
- GridRowCells
- GridRowCount
- GridSaveAltIcon
- GridScrollArea
- GridSearchIcon
- GridSelectedRowCount
- GridSeparatorIcon
- GridStickyContainer
- GridTableRowsIcon
- GridToolbar
- GridToolbarColumnsButton
- GridToolbarContainer
- GridToolbarDensitySelector
- GridToolbarExport
- GridToolbarFilterButton
- GridTripleDotsVerticalIcon
- GridViewHeadlineIcon
- GridViewStreamIcon
- GridViewport
- GridWindow
- HideGridColMenuItem
- SortGridMenuItems


// Cell renderers
- renderActionsCell // TODO: Remove from export
- renderEditInputCell // TODO: Remove from export
- renderEditSingleSelectCell // TODO: Remove from export


// Selectors
- activeGridFilterItemsSelector // TODO: Rename
- allGridColumnsFieldsSelector // TODO: Rename
- allGridColumnsSelector // TODO: Rename
- filterGridColumnLookupSelector // TODO: Rename
- filterGridItemsCounterSelector // TODO: Rename
- filterableGridColumnsIdsSelector // TODO: Rename
- filterableGridColumnsSelector // TODO: Rename
- gridColumnLookupSelector
- gridColumnMenuSelector
- gridColumnReorderDragColSelector
- gridColumnReorderSelector
- gridColumnResizeSelector
- gridColumnsMetaSelector
- gridColumnsSelector
- gridColumnsTotalWidthSelector
- gridDateFormatter
- gridDateTimeFormatter
- gridDensityHeaderHeightSelector
- gridDensityRowHeightSelector
- gridDensitySelector
- gridDensityValueSelector
- gridEditRowsStateSelector
- gridFilterModelSelector
- gridFocusCellSelector
- gridFocusColumnHeaderSelector
- gridFocusStateSelector
- gridPaginatedVisibleSortedGridRowIdsSelector
- gridPaginationSelector
- gridPreferencePanelStateSelector
- gridRenderingSelector
- gridResizingColumnFieldSelector
- gridRowCountSelector
- gridRowsLookupSelector
- gridRowsStateSelector
- gridScrollSelector
- gridScrollbarStateSelector
- gridSelectionStateSelector
- gridSortColumnLookupSelector
- gridSortModelSelector
- gridTabIndexCellSelector
- gridTabIndexColumnHeaderSelector
- gridTabIndexStateSelector
- gridViewportSizeStateSelector
- selectedGridRowsCountSelector // TODO: Rename
- selectedGridRowsSelector // TODO: Rename
- selectedIdsLookupSelector // TODO: Rename
- sortedGridRowIdsSelector // TODO: Rename
- sortedGridRowsSelector // TODO: Rename
- unorderedGridRowIdsSelector // TODO: Rename
- unorderedGridRowModelsSelector // TODO: Rename
- visibleGridColumnsLengthSelector // TODO: Rename
- visibleGridColumnsSelector // TODO: Rename
- visibleGridRowCountSelector // TODO: Rename
- visibleGridRowsStateSelector // Removed in #2782
- visibleSortedGridRowIdsSelector // TODO: Rename
- visibleSortedGridRowsAsArraySelector // TODO: Rename
- visibleSortedGridRowsSelector // TODO: Rename


// Contexts
- GridApiContext // TODO: Remove from export in v6 (we should only export `useGridApiContext` to read the context and a have a component that properly set the provider, see #2522)


// Feature Enums
- GridCellModes
- GridDensityTypes // TODO: Rename "GridDensities" and remore `GridDensity` type
- GridEditModes
- GridEvents
- GridFeatureModeConstant // TODO: Rename GridFeatureModes and remove `GridFeatureMode` type
- GridLinkOperator
- GridPreferencePanelsValue
- GridRowModes


// Localizations
- arSD
- bgBG
- csCZ
- deDE
- elGR
- enUS
- esES
- faIR
- frFR
- itIT
- jaJP
- koKR
- nlNL
- plPL
- plPLGrid // Fixed in #2791: Remove from export
- ptBR
- ruRU
- ruRUGrid // Fixed in #2791: Remove from export
- skSK
- skSKGrid // Fixed in #2791: Remove from export
- trTR
- ukUA
- ukUAGrid // Fixed in #2791: ReRemove from export
- viVN
- zhCN
- zhCNGrid // Fixed in #2791: Remove from export


// Utility functions
- checkGridRowIdIsValid
- getDataGridUtilityClass
- getGridColDef
- getGridDefaultColumnTypes
- getInitialGridColumnReorderState // Fixed in #2782: Remove from export + Stop use internally
- getInitialGridColumnResizeState // Fixed in #2782: Remove from export + Stop use internally
- getInitialGridColumnsState // Fixed in #2782: Remove from export + Stop use internally 
- getInitialGridDensityState // Fixed in #2782: Remove from export + Stop use internally 
- getInitialGridFilterState // Fixed in #2782: Rename getDefaultGridFilterModel 
- getInitialGridRenderingState // Fixed in #2782: Remove from export + Stop use internally 
- getInitialGridRowState // Fixed in #2782: Remove from export + Stop use internally 
- getInitialGridSortingState // Fixed in #2782: Remove from export + Stop use internally 
- getInitialPaginationState // Fixed in #2782: Remove from export + Stop use internally 
- getInitialVisibleGridRowsState // Fixed in #2782: Remove from export + Stop use internally 


// Utility hooks
- useGridApi
- useGridApiContext
- useGridApiEventHandler
- useGridApiMethod
- useGridApiOptionHandler
- useGridApiRef
- useGridLogger
- useGridRootProps
- useGridScrollFn
- useGridSelector
- useGridSlotComponentProps
- useGridState
- useNativeEventListener
- useProcessedProps


// Class utilies
- gridClasses
- gridPanelClasses

@m4theushw
Copy link
Member

m4theushw commented Oct 6, 2021

For the components, we can stop exporting without doubt the icons and inner components used in the virtualization.

- DataGridPro (or DataGrid for `x-data-grid`)
- GridActionsCell
- GridActionsCellItem
- GridAddIcon // TODO remove
- GridArrowDownwardIcon // TODO remove
- GridArrowUpwardIcon // TODO remove
- GridAutoSizer // TODO remove
- GridBody // TODO remove
- GridCell
- GridCellCheckboxForwardRef
- GridCellCheckboxRenderer
- GridCheckCircleIcon // TODO remove
- GridCheckIcon // TODO remove
- GridCloseIcon // TODO remove
- GridColumnHeaderItem
- GridColumnHeaderMenu
- GridColumnHeaderSeparator
- GridColumnHeaderSortIcon // TODO remove
- GridColumnHeaderTitle
- GridColumnHeadersItemCollection // TODO remove
- GridColumnIcon // TODO remove
- GridColumnMenu
- GridColumnMenuContainer
- GridColumnsContainer
- GridColumnsHeader // TODO remove
- GridColumnsMenuItem // TODO remove
- GridColumnsPanel
- GridDataContainer // TODO remove
- GridDragIcon // TODO remove
- GridEditInputCell
- GridEditSingleSelectCell
- GridEmptyCell // TODO remove
- GridErrorHandler // TODO remove
- GridFilterAltIcon // TODO remove
- GridFilterForm
- GridFilterInputValue
- GridFilterListIcon // TODO remove
- GridFilterMenuItem
- GridFilterPanel
- GridFooter
- GridFooterContainer
- GridFooterPlaceholder
- GridHeader
- GridHeaderCheckbox
- GridHeaderPlaceholder
- GridLoadIcon // TODO remove
- GridLoadingOverlay
- GridMenu
- GridMenuIcon // TODO remove
- GridMoreVertIcon // TODO remove
- GridNoRowsOverlay
- GridOverlay // TODO remove
- GridOverlays // TODO remove
- GridPagination
- GridPanel
- GridPanelContent
- GridPanelFooter
- GridPanelHeader
- GridPanelWrapper
- GridPreferencesPanel
- GridRenderingZone // TODO remove
- GridRoot // TODO remove
- GridRow
- GridRowCells // TODO remove
- GridRowCount
- GridSaveAltIcon // TODO remove
- GridScrollArea // TODO remove
- GridSearchIcon // TODO remove
- GridSelectedRowCount
- GridSeparatorIcon // TODO remove
- GridStickyContainer
- GridTableRowsIcon // TODO remove
- GridToolbar
- GridToolbarColumnsButton
- GridToolbarContainer
- GridToolbarDensitySelector
- GridToolbarExport
- GridToolbarFilterButton
- GridTripleDotsVerticalIcon // TODO remove
- GridViewHeadlineIcon // TODO remove
- GridViewStreamIcon // TODO remove
- GridViewport // TODO remove
- GridWindow // TODO remove
- HideGridColMenuItem
- SortGridMenuItems

@flaviendelangle flaviendelangle changed the title [RFC] Clean XGrid and DataGrid modules exports [RFC] Clean x-data-grid and x-data-grid-pro modules exports Oct 13, 2021
@flaviendelangle flaviendelangle changed the title [RFC] Clean x-data-grid and x-data-grid-pro modules exports [RFC] Clean x-data-grid and x-data-grid-pro exports Oct 13, 2021
@flaviendelangle
Copy link
Member Author

I will do the component cleanup just after the virtualization PR merge to avoid further conflicts.

@flaviendelangle
Copy link
Member Author

I am stopping the effort for v5, I will resume it when we will start working on v6 alpha.

@oliviertassinari oliviertassinari added the component: data grid This is the name of the generic UI component, not the React module! label Jan 29, 2022
@oliviertassinari oliviertassinari added the RFC Request For Comments label Aug 12, 2022
@flaviendelangle
Copy link
Member Author

Closing this one, see #3287 for the v6 cleaning

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: data grid This is the name of the generic UI component, not the React module! new feature New feature or request RFC Request For Comments
Projects
None yet
Development

No branches or pull requests

3 participants