[data grid] Reduce the use of useGridSelector in root-level hooks

[lauri865]

• I have followed (at least) the PR section of the contributing guide.

As discussed in #15986 (comment)

The use of useGridSelector should be avoided whenever possible in root level (feature) hooks, as it sends the whole grid root and all sibling hooks to re-evaluate on every time selecto state changes. There's many instances, where useGridSelector values are passed to useCallback, which re-creates callback functions and event listeners on state changes. If the callbacks are indeed callbacks, the state they access is identical to just evaluating the selector within the callback itself.

This instance is a bit more difficult to refactor for the time being, but will be very easy once #15666 gets merged as it'll be refactored as a pure selector then:

const initialCurrentPageRows = useGridVisibleRows(apiRef, props).rows;

The only instances where useGridSelector is valid truly valid and necessary are useGridDimensions and useGridRowsMeta as they need to perform calculations in response to many different parts of the state. If those were to be optimised further, they could be moved to a separate component, so they wouldn't affect all the other hooks any time the state they depend on changes.

Two other instances that were difficult to refactor currently:

mui-x/packages/x-data-grid-pro/src/hooks/features/dataSource/useGridDataSource.ts

Line 91 in b061f55

const groupsToAutoFetch = useGridSelector(apiRef, gridRowGroupsToFetchSelector);

mui-x/packages/x-data-grid/src/hooks/features/columnResize/useGridColumnResize.tsx

Line 135 in b061f55

function useColumnVirtualizationDisabled(apiRef: React.MutableRefObject<GridPrivateApiCommunity>) {

But they are pretty minor.

However, if we could get rid of them completely, I think it would make for a good convention not to allow selector hooks in root-level feature hooks. Makes it easier to write more isolated features, and safer to introduce experimental features. Alternative is:
#15986 (comment)

[mui-bot]

Deploy preview: https://deploy-preview-16001--material-ui-x.netlify.app/

Generated by 🚫 dangerJS against 68286c2

[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.

[lauri865]

Out of curiosity, I moved useGridDimensions and useGridRowsMeta into a separate component (locally, haven't pushed these changes, but happy to do so if there's interest).

It broke a few tests, most notably anything related to tree data. Revealing that `useGridVisibleRows' is a problematic hook – it currently only works due to side-effects of other state updating it seems.

Again, it will be an easy fix once #15666 gets merged (fyi @romgrk).

But this confirms my hunch that avoiding useGridSelector in root level feature hooks is probably a good convention to have – not only for performance considerations, but also forces one to write more isolated code and avoids cases of things working due to side-effects.

[lauri865]

I'm not sure if this function is legacy, but I cannot see any case where the useColumnVirtualizationDisabled function could hit anything but this code path after invoking apiRef.current.unstable_setColumnVirtualization(false):
https://github.com/mui/mui-x/blob/61d8511407190a7f1b7a4f4a672ffe3ee3b33342/packages/x-data-grid/src/hooks/features/columnResize/useGridColumnResize.tsx#L149C7-L151C8

So, I simplified it, and got rid of the last root-level useGridSelector. Technically any async function that is awaited could do the trick, but for semantics and to avoid future breaking, it's better to check the selector anyways. Correct me if I'm wrong, but tested with virtualized columns and everything worked with the new implementation.

[github-actions]

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

[lauri865]

@MBilalShafi, out of curiosity, why did you change the below from an event (was previously triggered by rowsSet I believe) to a useEffect? Just trying to understand the limits of my suggestion.

mui-x/packages/x-data-grid-pro/src/hooks/features/dataSource/useGridDataSourceBase.ts

Lines 403 to 413 in a2c7b00

	React.useEffect(() => {
	if (
	groupsToAutoFetch &&
	groupsToAutoFetch.length &&
	scheduledGroups.current < groupsToAutoFetch.length
	) {
	const groupsToSchedule = groupsToAutoFetch.slice(scheduledGroups.current);
	nestedDataManager.queue(groupsToSchedule);
	scheduledGroups.current = groupsToAutoFetch.length;
	}
	}, [apiRef, nestedDataManager, groupsToAutoFetch]);