[core] Initialize states before feature hooks

[m4theushw]

Problem

I can't call useCurrentPageRows inside useGridRows because it needs the pagination state which is not initialized yet when this hook is called. Another situation was when developing the master/detail where I needed to access the row IDs selector inside a visibleRowsSet listener but I couldn't because the hook would need to be called before useGridColumns but after useGridParamsApi. In the end I created a separate useGridDetailPanelCache hook only with the function that had the dependencies. The same happened with the variable row height because it depended on a few sub-states not yet initialized when useGridRows is called.

Proposed solution

Approach 3 from #2293

Each hook now exports a state initializer function. These functions are all called at once before any feature hook is called (e.g. useGridSelection). This way, when a feature hook is called it already has the state fully initialized so useGridSelector works. Pre-processors registered in the hydrateColumns group need to be called even before the state initializers because when the columns state is initialized it tries to hydrate the columns. As solution, they were moved to a hook useXXXPreProcessors.

I only created state initializers for the most critical states. The main goal is to be able to use useCurrentPageRows inside useGridRows to allow to create a apiRef.current.getRowIndex that works and remove the useMemo from https://github.com/mui/mui-x/pull/3848/files#diff-18800fbf9ede54a6e064cdd6278c6c551b82d8ab4c0ded647cdba1ff364364c2R45.

The remaining state initializers will be done in a follow-up.

[mui-bot]

These are the results for the performance tests:

Test case	Unit	Min	Max	Median	Mean	σ
Filter 100k rows	ms	213	429.8	307	321.48	82.722
Sort 100k rows	ms	422.6	889.5	707.6	669.78	151.48
Select 100k rows	ms	132.6	282.4	230.4	215.96	53.726
Deselect 100k rows	ms	87.6	254.4	254.4	177.42	70.695

Generated by 🚫 dangerJS against bfa02cd

[flaviendelangle]

See #2293 where we discussed the various approaches

It make sense that we are now at a point where registering the state directly in the hook causes problems.

[m4theushw]

This workaround is necessary because the column pinning pre-processor needs to be called before hydrating the columns but at the same time it needs to react to state changes. Maybe makes sense to add apiRef.current.isStateInitialized and put everything inside a useMemo.

[m4theushw]

The pre-processors registered inside these hooks don't react to state changes to update the columns. Instead, they are registered again. If moved to a useGridTreeDataPreProcessors hook the same code would have to be in two places (inside the new hook and also in useGridTreeData). This can be improved I think.

mui-x/packages/grid/_modules_/grid/hooks/features/treeData/useGridTreeData.tsx

Lines 53 to 55 in 23cf2fe

	const updateRowGrouping = React.useCallback(() => {
	if (!props.treeData) {
	return apiRef.current.unstable_registerRowGroupsBuilder('treeData', null);

[m4theushw]

Don't need to be in the state rows cache. They could be refs.

[flaviendelangle]

Before this PR, they were not in the state (if by state you mean GridState) but in the internal cache of useGridRows which is a ref.
Looks like you added them in the state to be able to access them in useGridRows and rowsStateInitializer.
Maybe it's a necessary costs of this new implementation.

The more we fragment a feature logic into several hooks and methods, the more this problem will occur.
And if at some point our state becomes a real "react state" (and causes rerenders), we won't be able to store those on the state because they update way to frequently.

Maybe we could introduce an apiRef.current.caches collection which would always behave as a ref, and never cause re-renders.

[m4theushw]

I'm was referring to state because it's now in the "state", but thinking it like the old internal cache also works. I didn't understand why these two values were inside an object while they could be standalone refs. I'll open another PR to keep them only inside this hook, not leaking to the state.

[m4theushw]

Keeping the props in the state was causing a dependency cycle because it depends on DataGridProcessedProps. I can't understand the reason why it was being stored since we always have access to the props.

[flaviendelangle]

I stored them in the internal cache (which was not in the state before this PR) to avoid adding them as dependencies of a useEffect by mistake which could cause useless tree generations.

But it is exactly the same as adding an eslint-disable-line react-hooks/exhaustive-deps on those useEffect.

And after a quick look on this hook, it seems that we now have way less useEffect than before so this duplication issue should not be a big problem if we stay vigilent.

[m4theushw]

It's rare when we need to use eslint-disable-line react-hooks/exhaustive-deps in the codebase. Lets see if it doesn't cause bugs keeping that way.

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[flaviendelangle]

I did a few tests and for me everything is working fine
I agree that we can improve this management later. Things are starting to get clearer around these feature lifecycles.

If you want to merge this one, I can rebase #3965 after