[core] Each feature hook should initialize synchronously its state

[flaviendelangle]

Part of #2231

Goals

Why have each hook initialize its state ?

To better isolate XGrid only features (to reduce the size of the DataGrid bundle et don't put non-MIT code in it)
To allow a headless version of the Grid that would not import the unused features #1016

Why initialize it synchronously

The current state initialization is asynchronous.
On the first render, we have the default state.
Then each hook can update it to put values from the props.

For instance:

// useGridPageSize.ts
export const useGridPageSize = (
  apiRef: GridApiRef,
  props: Pick<GridComponentProps, 'pageSize' | 'onPageSizeChange' | 'autoPageSize'>,
) => {
  [...]

  React.useEffect(() => {
    const autoPageSize = containerSizes?.viewportPageSize;
    const prevPageSize = apiRef.current.getState().pagination.pageSize;

    let pageSize = prevPageSize;

    if (props.pageSize != null) {
      pageSize = props.pageSize;
    } else if (props.autoPageSize) {
      pageSize = autoPageSize ?? 0;
    }

    if (pageSize !== prevPageSize) {
      if (props.autoPageSize) {
        apiRef.current.publishEvent(GRID_PAGE_SIZE_CHANGE, autoPageSize);
      }

      setGridState((state) => ({
        ...state,
        pagination: {
          ...state.pagination,
          pageSize,
        },
      }));
      forceUpdate();
    }
  }, [
    apiRef,
    setGridState,
    forceUpdate,
    visibleRowCount,
    props.autoPageSize,
    props.pageSize,
    containerSizes?.viewportPageSize,
  ]);

  [...]
};

Which means on the 1st render, we have the default value even on hooks that are below this one.

Solutions

Approach 1

Pro

• Simple
• No change in useDataGridComponent.ts / useXGridComponent.ts

Con

• On the 1st render, the hooks won't have access to the state of the hooks below them (impossible to reliably use selectors in the render)

const useGridPageSize = (api: GridApiRef, props: Pick<GridComponentProps, 'pageSize, 'onPageSizeChange'>)  => {
  useGridStateInit(apiRef, state => ({ ...state, pagination: { page:Size props.pageSize ?? 0 }}));
}

const useGridPage = (api: GridApiRef, props: Pick<GridComponentProps, 'page, 'onPageChange'>)  => {
  useGridStateInit(state => ({ ...state, pagination: { ...state.pagination!, page: props.page }}));
}

const useGridStateInit = (apiRef, callback: (state: Partial<GridState>): Partial<GridState>)) => {
  const isInitialized = React.useRef(false);
  const [, setGridState] = useGridState(apiRef);

  if (!isInitialized.current) {
    setGridState(callback);
  }
} 

---

Approach 2

Each hook can export a state initializer. All hooks are called in a single hook taking care of the state initialization and then running the hooks themselves.

Pro

• No change in useDataGridComponent.ts / useXGridComponent.ts for the state initialization when adding a new feature
• When running the 1st feature hook, we already have a valid state with all the props initialized
• It will be easy in the future to allow the user to pass manual feature hook and just add them to the list passed to useGridPlugins

Con

• Deeper changes

// useGridPageSoze.ts
export const useGridPageSoze: GridPlugin<'pageSize', 'onPageSizeChange'> = (api, props) => {

}

useGridPage.initState = (state, props) => ({ ...state, pagination: { page: props.pageSize ?? 100 }})

// useGridPage.ts
export const useGridPage: GridPlugin<'page', 'onPageChange'> = (api, props) => {

}

useGridPage.initState = (state, props) => ({ ...state, pagination: { ...state.pagination!, page: props.page ?? 0 }})

// GridPlugin.ts
export type GridPlugin<PropKeys extends keyof GridComponentProps> = {
  (apiRef: GridApiRef, props: Pick<GridComponentProps, PropKeys>): void;
  initState?: (state: Partial<GridState>, props: Pick<GridComponentProps, PropKeys>) => Partial<GridState>
} 

// useGridPlugins.ts
// The plugin must change from a render to another to respect the rule of hooks
const useGridPlugins = (plugins: GridPlugin[]): void => {
  const isInitialized = React.useRef(false)

  if (!isInitialized.current) {
    isInitialized.current = true
    apiRef.current.state = plugins.reduce(
      (acc, plugin) => plugin?.initState(acc) ?? acc, 
      {} as Partial<GridState>
    ) 
    apiRef.current.publishEvent(GridEvents.initState, state)
  }

  for (const plugin in plugins) {
    plugin(apiRef, props);
  }
} 

const X_GRID_PLUGINS: GridPlugin[] = [useGridPageSize, useGridPage]

// XGrid.tsx
const XGrid = (inProps: XGridProps) => {
  const apiRef = useGridApiRef(inProps.apiRef);
  const props = useThemeProps({ props: inProps, name: 'MuiDataGrid' });

  useGridPlugins(X_GRID_PLUGINS)
}

---

Approach 3

Each hook can export a state initializer. The useXGridComponents and useDataGridComponents are then responsible or initializing the state by manually calling each of those state initializer.

Pro

• When running the 1st feature hook, we already have a valid state with all the props initialized
• Simpler to build and less technical overhead than Approach n°2

Con

• When adding a new feature, we need to add it to useDataGridComponent.ts / useXGridComponent.ts for the hook call AND for the state initialization (more error prone)

// useGridPageSize.ts
export const useGridPage = (api: GridApiRef, props: Pick<GridComponentProps, 'page, 'onPageChange'>) => {
}

export const initPageSizeState = (state, props) => ({ ...state, pagination: { page: props.page ?? 0 }})

// useGridPage.ts
export const useGridPage = (api: GridApiRef, props: Pick<GridComponentProps, 'page, 'onPageChange'>) => {
}

export const initPageState = (state, props) => ({ ...state, pagination: { ...state.pagination!, page: props.page ?? 0 }})

// XGrid.tsx
const XGrid = (inProps: XGridProps) => {
  const apiRef = useGridApiRef(inProps.apiRef);
  const props = useThemeProps({ props: inProps, name: 'MuiDataGrid' });

  const isInitialized = React.useRef(false)

  if (!isInitialized.current) {
    isInitialized.current = true
   
    let state: Partial<GridState> = {};
    state = initPageSizeState(state, props);
    state = initiPageState(state, props);
    apiRef.current.state = state as GriState;
    apiRef.current.publishEvent(GridEvents.initState, state);
  }

  useGridPageSize(apiRef, props);
  useGridPage(apiRef, props);
}

Side effects

If we initialize the state synchronously, we will have to rework a bit the control state API.
It will not cause public behavior change.

[oliviertassinari]

If we initialize the state synchronously, we will have to rework a bit the control state API.

I would even say that it should fix:

https://github.com/mui-org/material-ui-x/blob/49639ee9924517b5f1aee3d8233f0ddd669c8fdc/packages/grid/_modules_/grid/hooks/features/core/useGridControlState.ts#L33-L35

Regarding the options. I would personally try 1 first, see if it's flying, otherwise fallback to 2. Option 3 seems to force to have to handle each plugin manually,

[flaviendelangle]

I agree on Option 3. I had to propose it since its a lighter version of Option 2 on the implementation but it's not viable for me.

For Option 1 to work, we need to be even stricter about the order of the hooks.
I would be in favor of declaring explicitely in each hook which hooks it requires.

Maybe through the JSDoc

/**
 * @requires usePageSize
 **/
const usePage = () => { ... } 

Or in TypeScript is we want to do a validation in the future for custom use hooks, but it may be overkill for now.

[flaviendelangle]

I'm doing a first working draft of the state synchronous initialization
Overall it seems to be working fine and most of the issue I struggle fixing are not blocking.

Feature hook structure

export const useGridPage = (
  apiRef: GridApiRef,
  props: Pick<GridComponentProps, 'page' | 'onPageChange' | 'rowCount'>,
) => {
  // 1. LOGGER
  const logger = useGridLogger(apiRef, 'useGridPage');

  // 2. STATE INITIALIZATION
  // We initialize the state before 3. because if the hook is relying on selectors of its own state, it would crash
  useGridStateInit(apiRef, (state) => ({
    ...state,
    pagination: {
      ...state.pagination!,
      page: 0,
      pageCount: getPageCount(props.rowCount ?? 0, state.pagination!.pageSize!),
      rowCount: props.rowCount ?? 0,
    },
  }));

  // 3. STATE SELECTORS AND UPDATER
  const [, setGridState, forceUpdate] = useGridState(apiRef);
  const visibleRowCount = useGridSelector(apiRef, visibleGridRowCountSelector);

  // 4. CONTROL STATE REGISTRATION
  // We synchronously define the control state whereas before it was asynchronously defined
  useGridRegisterControlState(apiRef, {
    stateId: 'page',
    propModel: props.page,
    propOnChange: props.onPageChange,
    stateSelector: (state) => state.pagination.page,
    changeEvent: GridEvents.pageChange,
  });

  // 5. API METHODS
  const setPage = React.useCallback<GridPageApi['setPage']>(
    (page) => {
      logger.debug(`Setting page to ${page}`);

      setGridState((state) => ({
        ...state,
        pagination: applyValidPage({
          ...state.pagination,
          page,
        }),
      }));
      forceUpdate();
    },
    [setGridState, forceUpdate, logger],
  );

  const pageApi: GridPageApi = {
    setPage,
  };

  useGridApiMethod(apiRef, pageApi, 'GridPageApi');

  // 6. UPDATE EFFECTS
  // A major future evolution would be to limit as much as possible this pattern to avoid `useEffect` cascade.
  React.useEffect(() => {
    setGridState((state) => {
      const rowCount = props.rowCount !== undefined ? props.rowCount : visibleRowCount;
      const pageCount = getPageCount(rowCount, state.pagination.pageSize);

      const page = props.page == null ? state.pagination.page : props.page;

      return {
        ...state,
        pagination: applyValidPage({
          ...state.pagination,
          page,
          rowCount,
          pageCount,
        }),
      };
    });
    forceUpdate();
  }, [setGridState, forceUpdate, visibleRowCount, props.rowCount, props.page, apiRef]);

  // 7. EVENT CALLBACKS
  const handlePageSizeChange = React.useCallback(
    (pageSize: number) => {
      setGridState((state) => {
        const pageCount = getPageCount(state.pagination.rowCount, pageSize);

        return {
          ...state,
          pagination: applyValidPage({
            ...state.pagination,
            pageCount,
            page: state.pagination.page,
          }),
        };
      });

      forceUpdate();
    },
    [setGridState, forceUpdate],
  );

  useGridApiEventHandler(apiRef, GridEvents.pageSizeChange, handlePageSizeChange);
};

Points to clarify

For the following points, the implementation I did seems non-optimal.
If you have better ideas.

Some state initialization are too complicated to be done at the start of their hook

For instance, the filter state is applied like this

  React.useEffect(() => {
    // [...]
    const oldFilterModel = apiRef.current.state.filter;
    if (props.filterModel !== undefined && props.filterModel !== oldFilterModel) {
      logger.debug('filterModel prop changed, applying filters');
      setGridState((state) => ({
        ...state,
        filter: props.filterModel,
      }));
      apiRef.current.applyFilters();
    }
  }, [apiRef, logger, props.filterModel, setGridState]);

I added the filter: props.filterModel inside the initialization hook, but the apiRef.current.applyFilters can hardly be done at the start of the file because it needs like half of the hook code

So I did the following

export const useGridFilter = (apiRef, props): void => {
  const logger = useLogger('useGridFilter');

  useGridStateInit(apiRef, (state) => ({
    ...state,
    filter: props.filterModel ?? getInitialGridFilterState(),
    visibleRows: {
      visibleRowsLookup: {},
    },
  }));

  // ... all the rest of the hook code

  useFirstRender(() => apiRef.current.applyFilters());
}

With useFirstRender being a small wrapper to only execute the callback synchronously during the 1st render

export const useFirstRender = (callback: () => void) => {
  const isFirstRender = React.useRef(true);

  if (isFirstRender.current) {
    isFirstRender.current = false;
    callback();
  }
};

Which means, if we have synchronous access to the state during the 1st render, virtualRows won't be initialized.
They may be a better solution and it could be an issue when trying synchronous state updates, but for now it's working fine.
Note that the state initialization is still synchronous, so any hook after useFilter can have a selector and access the visibleRows.

Code duplication for state initialization

Here is the code to init the useGridColumns state

  useGridStateInit(apiRef, (state) => {
    const hydratedColumns = hydrateColumnsType(
      props.columns,
      props.columnTypes,
      apiRef.current.getLocaleText,
      props.checkboxSelection,
    );

    const columns = upsertColumnsState(hydratedColumns);
    let newColumns: GridColumns = columns.all.map((field) => columns.lookup[field]);
    newColumns = hydrateColumnsWidth(newColumns, 0);

    const columnState: GridColumnsState = {
      all: newColumns.map((col) => col.field),
      lookup: newColumns.reduce((acc, col) => {
        acc[col.field] = col;
        return acc;
      }, {}),
    };

    return {
      ...state,
      columns: columnState,
    };
  });

It's totally a duplicate of the useEffect.
For this kind of code duplication, a small refactoring should solve the issue.

The useEffect with setGridState in it uselessly runs after the 1st render

For instance in useGridPage, we set the state synchronously, but then we have a useEffect that tracks the props and reapply a the exact same pagination.
This cause an additional rerender which we could avoid.

I see two solutions here

1. We create a hook useUpdate or useEffectNotFirstRender (name to be improved) that is just a useEffect skipping the 1st render.

2. We manually check that the pagination we created is not equal to the one we have in state. For pagination it's just comparing 3 values, for hooks like useGridColumns it would mean keeping the input props in state to compare them because the output object is a lot more complex to compare.

[flaviendelangle]

@oliviertassinari @m4theushw @DanailH when you have some time to take a look at the hook example above and the points to be improved about the typical structure of a feature hook.

https://github.com/flaviendelangle/material-ui-x/pull/1/files
I'll open this PR in the mui-org repo to start talk about the implementation and the impacts on specific feature hooks.

The main issue that we must address before merging anything is Some components uses selectors of useGridReorder which is a pro-only hook. .
We really have to find a way for optional features to pass elements to the components without having the components importing the selectors.