mui / mui-x

MUI X: Build complex and data-rich applications using a growing list of advanced React components, like the Data Grid, Date and Time Pickers, Charts, and more!
https://mui.com/x/
4.08k stars 1.26k forks source link

[RFC] Codebase structure for hooks #2608

Open flaviendelangle opened 3 years ago

flaviendelangle commented 3 years ago

The goal of this RFC is to discuss about a clean code structure for our features to improve the following subjects:

What is a feature hook ?

I am using the term feature hook because most of them are currently in the hooks/features folder, but we may call them plugin if it makes more sense.

A feature hook is a hook which implements a given feature for the DataGrid or DataGridPro. It is called in useDataGridComponent and useDataGridProComponent and would be the main blocks of a hook-only API.

It is usually performing some of the following actions:

Not all feature hooks perform all of those actions. For instance useGridKeyboard is listening to events but does not add any method to the GridApi and is not managing any dedicated sub-state.

Feature hook in a headless implementation

See #1016

Proposal: 1 sub-state = 1 feature hook = 1 folder

I propose we enforce two rules to have a cleaner framework to work on.

A feature hook must never handle several sub states

useGridFocus

-useGridStateInit(apiRef, (state) => ({
-  ...state,
-  focus: { cell: null, columnHeader: null },
-  tabIndex: { cell: null, columnHeader: null },
-}));
+useGridStateInit(apiRef, (state) => ({
+  focus: {
+    value: { cell: null, columnHeader: null }, // key naming to discuss
+    tabIndex: { cell: null, columnHeader: null },
+  },
+})

useGridFilter

Modified in #2572

-useGridStateInit(apiRef, (state) => ({
-  ...state,
-  filter: props.filterModel ?? getDefaultGridFilterModel(),
-  visibleRows: {
-    visibleRowsLookup: {},
-    visibleRows: null,
-  },
-}));
+useGridStateInit(apiRef, (state) => ({
+  ...state,
+  filter: {
+    filterModel: props.filterModel ?? getDefaultGridFilterModel(),
+    visibleRowsLookup: {},
+    visibleRows: null,
+  },
+}));

A sub-state should never be updated by several feature hooks

If a sub-state is modified by two very intricate feature hooks, maybe we should add a parent feature hook that calls both of them. This would clarify the feature list in useDataGridComponent to really have major features and not implementation details.

Call a single useGridPagination hook in useDataGridComponent

const useGridPagination = (apiRef, props) => {
  useGridPageSize(apiRef, props);
  useGridPage(apiRef, props);
} 
// useDataGridComponent.ts
-useGridPageSize(apiRef, props);
-useGridPage(apiRef, props);
+useGridPagination(apiRef, props);

Call a single useGridVirtualization hook in useDataGridComponent

const useGridVirtualization = (apiRef, props) => {
  useGridNoVirtualization(apiRef, props);
  useGridVirtualizationEnabled(apiRef, props);
} 
// useDataGridComponent.ts
-useGridNoVirtualization(apiRef, props);
-useGridVirtualization(apiRef, props);
+useGridVirtualization(apiRef, props);

// useGridVirtualization.ts
-export const useGridVirtualization = (
+export const useGridVirtualizationEnabled = (

Proposal: Group technical hooks inside useGridInitialization

DONE

We currently have a long list of hook calls in useDataGridComponent, in part because their are a lot of small technical hooks like useGridLoggerFactory, useApi, useGridControlStateManager, useLocaleText.

I think we can consider these hooks to be mandatory for any Grid implementation, and therefore group their calls into a single exported hooks.

The useDataGridComponent would then look something like this

export const useDataGridComponent = (apiRef, props) => {
  useGridInitialization(apiRef, props)

  // Features
  useGridResizeContainer(apiRef, props);
  useGridColumns(apiRef, props);
  useGridRows(apiRef, props);
  useGridParamsApi(apiRef, props);
}

Proposal: Structure of the hooks folder

PARTIALLY DONE (still have to split x-data-grid and x-data-grid-pro)

The current hooks/core / hooks/utils and hooks/features folders match pretty much what I describe bellow but for me some hooks are not in the correct place

packages/grid/data-grid

- hooks
  // All the hooks called in other places than just `useDataGridComponent` or `useDataGridProComponent`
  // These hooks should be correctly documented as most of them should be public and frequently used in our codebase
  - utils
    - useGridApiMethod.ts
    - useGridApiEventHandler.ts
    - useGridSelector.ts
    - useGridRegisterControlState.ts
    - useGridStateInit.ts
    - ...

  // All the feature hooks called in `useDataGridComponent` and `useDataGridProComponent`
  // Each folder exports a single hook called by the `useDataGridComponent` file
  - features 
    - density
      - useGridDensity.ts
      - densitySelector.ts
      - densityState.ts

    - focus  
      - useGridFocus.ts
      - focusSelector.ts
      - focusState.ts

    - ...

  // All the hooks called by useGridInitialization, only useGridInitialization is exported from this folder
  - core
    - useGridInitialization.ts
    - useGridApi.ts // the current useApi.ts
    - useGridControlStateManager.ts
    - useGridLoggerFactory.ts
    - ...

packages/grid/data-grid-pro

Only contains the features not available on DataGrid. It reexports all the features from @mui/x-data-grid

  // All the feature hooks called in `useDataGridProComponent`
  - features 
    - columnReorder
      - useGridColumnReorder.ts
      - columnReorderSelector.ts
      - columnReorderState.ts

    - columnResize
      - useGridColumnResize.ts
      - columnResizeSelector.ts
      - columnResizeState.ts

    - treeData
      - useGridTreeData.ts
      - treeDataSelector.ts
      - treeDataState.ts

    - ...
flaviendelangle commented 2 years ago

@mui-org/x I you have feedback on this I would like to do the bundle clean separation in a few weeks :+1: