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

[DataGrid] Use new selectors #15200

Merged
merged 14 commits into from
Dec 11, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,7 @@ const GROUPING_COL_DEF_FORCED_PROPERTIES_DATA_SOURCE: Pick<
* TODO: Make this index comparator depth invariant, the logic should not be inverted when sorting in the "desc" direction (but the current return format of `sortComparator` does not support this behavior).
*/
const groupingFieldIndexComparator: GridComparatorFn = (v1, v2, cellParams1, cellParams2) => {
const model = gridRowGroupingSanitizedModelSelector(
cellParams1.api.state,
cellParams1.api.instanceId,
);
const model = gridRowGroupingSanitizedModelSelector({ current: cellParams1.api });
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A step towards #11440

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're passing api in a few places, we should probably consider replacing those with apiRef.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it couples more with #11440, let's consider it when working on that.


const groupingField1 = (cellParams1.rowNode as GridGroupNode).groupingField ?? null;
const groupingField2 = (cellParams2.rowNode as GridGroupNode).groupingField ?? null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import {
GridDataSourceGroupNode,
useGridSelector,
} from '@mui/x-data-grid';
import { useGridSelectorV8 } from '@mui/x-data-grid/internals';
import CircularProgress from '@mui/material/CircularProgress';
import { useGridRootProps } from '../hooks/utils/useGridRootProps';
import { useGridPrivateApiContext } from '../hooks/utils/useGridPrivateApiContext';
Expand Down Expand Up @@ -54,8 +53,8 @@ function GridTreeDataGroupingCellIcon(props: GridTreeDataGroupingCellIconProps)
const classes = useUtilityClasses(rootProps);
const { rowNode, id, field, descendantCount } = props;

const isDataLoading = useGridSelectorV8(apiRef, gridDataSourceLoadingIdSelector, id);
const error = useGridSelectorV8(apiRef, gridDataSourceErrorSelector, id);
const isDataLoading = useGridSelector(apiRef, gridDataSourceLoadingIdSelector, id);
const error = useGridSelector(apiRef, gridDataSourceErrorSelector, id);

const handleClick = (event: React.MouseEvent<HTMLButtonElement>) => {
if (!rowNode.childrenExpanded) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
gridPaginationModelSelector,
GridRowId,
} from '@mui/x-data-grid';
import { createSelector, createSelectorV8 } from '@mui/x-data-grid/internals';
import { createSelector } from '@mui/x-data-grid/internals';
import { GridStatePro } from '../../../models/gridStatePro';

const computeStartEnd = (paginationModel: GridPaginationModel) => {
Expand Down Expand Up @@ -36,7 +36,7 @@ export const gridDataSourceLoadingSelector = createSelector(
(dataSource) => dataSource.loading,
);

export const gridDataSourceLoadingIdSelector = createSelectorV8(
export const gridDataSourceLoadingIdSelector = createSelector(
gridDataSourceStateSelector,
(dataSource, id: GridRowId) => dataSource.loading[id] ?? false,
);
Expand All @@ -46,7 +46,7 @@ export const gridDataSourceErrorsSelector = createSelector(
(dataSource) => dataSource.errors,
);

export const gridDataSourceErrorSelector = createSelectorV8(
export const gridDataSourceErrorSelector = createSelector(
gridDataSourceStateSelector,
(dataSource, id: GridRowId) => dataSource.errors[id],
);
3 changes: 2 additions & 1 deletion packages/x-data-grid/src/components/GridRow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,8 @@ const GridRow = React.forwardRef<HTMLDivElement, GridRowProps>(function GridRow(
const heightEntry = useGridSelector(
apiRef,
() => ({ ...apiRef.current.getRowHeightEntry(rowId) }),
objectShallowCompare,
undefined,
objectShallowCompare as (a: unknown, b: unknown) => boolean,
);

const style = React.useMemo(() => {
Expand Down
3 changes: 2 additions & 1 deletion packages/x-data-grid/src/components/cell/GridCell.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,8 @@ const GridCell = React.forwardRef<HTMLDivElement, GridCellProps>(function GridCe
result.api = apiRef.current;
return result;
},
objectShallowCompare,
undefined,
objectShallowCompare as (a: unknown, b: unknown) => boolean,
MBilalShafi marked this conversation as resolved.
Show resolved Hide resolved
);

const isSelected = useGridSelector(apiRef, () =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export const useGridStateInitialization = <PrivateApi extends GridPrivateApiComm
apiRef: React.MutableRefObject<PrivateApi>,
) => {
const controlStateMapRef = React.useRef<
Record<string, GridControlStateItem<PrivateApi['state'], any>>
Record<string, GridControlStateItem<PrivateApi['state'], any, any>>
>({});
const [, rawForceUpdate] = React.useState<PrivateApi['state']>();

Expand Down Expand Up @@ -40,10 +40,15 @@ export const useGridStateInitialization = <PrivateApi extends GridPrivateApiComm
const controlState = controlStateMapRef.current[stateId];
const oldSubState = controlState.stateSelector(
apiRef.current.state,
undefined,
apiRef.current.instanceId,
);

const newSubState = controlState.stateSelector(newState, apiRef.current.instanceId);
const newSubState = controlState.stateSelector(
newState,
undefined,
apiRef.current.instanceId,
);
if (newSubState === oldSubState) {
return;
}
Expand Down Expand Up @@ -82,7 +87,7 @@ export const useGridStateInitialization = <PrivateApi extends GridPrivateApiComm
if (updatedControlStateIds.length === 1) {
const { stateId, hasPropChanged } = updatedControlStateIds[0];
const controlState = controlStateMapRef.current[stateId];
const model = controlState.stateSelector(newState, apiRef.current.instanceId);
const model = controlState.stateSelector(newState, undefined, apiRef.current.instanceId);

if (controlState.propOnChange && hasPropChanged) {
controlState.propOnChange(model, {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ export const useGridFilter = (

const updateFilteredRows = React.useCallback(() => {
apiRef.current.setState((state) => {
const filterModel = gridFilterModelSelector(state, apiRef.current.instanceId);
const filterModel = gridFilterModelSelector(state, undefined, apiRef.current.instanceId);
romgrk marked this conversation as resolved.
Show resolved Hide resolved
const filterState = apiRef.current.getFilterState(filterModel);

const newState = {
Expand Down
10 changes: 5 additions & 5 deletions packages/x-data-grid/src/hooks/features/rows/useGridRows.ts
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ export const useGridRows = (
}

apiRef.current.setState((state) => {
const group = gridRowTreeSelector(state, apiRef.current.instanceId)[
const group = gridRowTreeSelector(state, undefined, apiRef.current.instanceId)[
GRID_ROOT_GROUP_ID
] as GridGroupNode;
const allRows = group.children;
Expand Down Expand Up @@ -573,10 +573,10 @@ export const useGridRows = (
const applyHydrateRowsProcessor = React.useCallback(() => {
apiRef.current.setState((state) => {
const response = apiRef.current.unstable_applyPipeProcessors('hydrateRows', {
tree: gridRowTreeSelector(state, apiRef.current.instanceId),
treeDepths: gridRowTreeDepthsSelector(state, apiRef.current.instanceId),
dataRowIds: gridDataRowIdsSelector(state, apiRef.current.instanceId),
dataRowIdToModelLookup: gridRowsLookupSelector(state, apiRef.current.instanceId),
tree: gridRowTreeSelector(state, undefined, apiRef.current.instanceId),
treeDepths: gridRowTreeDepthsSelector(state, undefined, apiRef.current.instanceId),
dataRowIds: gridDataRowIdsSelector(state, undefined, apiRef.current.instanceId),
dataRowIdToModelLookup: gridRowsLookupSelector(state, undefined, apiRef.current.instanceId),
});

return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ export const useGridSorting = (
};
}

const sortModel = gridSortModelSelector(state, apiRef.current.instanceId);
const sortModel = gridSortModelSelector(state, undefined, apiRef.current.instanceId);
const sortRowList = buildAggregatedSortingApplier(sortModel, apiRef);
const sortedRows = apiRef.current.applyStrategyProcessor('sorting', {
sortRowList,
Expand Down
80 changes: 10 additions & 70 deletions packages/x-data-grid/src/hooks/utils/useGridSelector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,34 +2,22 @@ import * as React from 'react';
import { fastObjectShallowCompare } from '@mui/x-internals/fastObjectShallowCompare';
import { warnOnce } from '@mui/x-internals/warning';
import type { GridApiCommon } from '../../models/api/gridApiCommon';
import type { OutputSelector, OutputSelectorV8 } from '../../utils/createSelector';
import type { OutputSelector } from '../../utils/createSelector';
import { useLazyRef } from './useLazyRef';
import { useOnMount } from './useOnMount';
import type { GridCoreApi } from '../../models/api/gridCoreApi';

function isOutputSelector<Api extends GridApiCommon, T>(
function isOutputSelector<Api extends GridApiCommon, Args, T>(
selector: any,
): selector is OutputSelector<Api['state'], T> {
): selector is OutputSelector<Api['state'], Args, T> {
return selector.acceptsApiRef;
}

type Selector<Api extends GridApiCommon, Args, T> =
| ((state: Api['state']) => T)
| OutputSelectorV8<Api['state'], Args, T>;
| OutputSelector<Api['state'], Args, T>;

// TODO v8: Remove this function
function applySelector<Api extends GridApiCommon, T>(
apiRef: React.MutableRefObject<Api>,
selector: ((state: Api['state']) => T) | OutputSelector<Api['state'], T>,
) {
if (isOutputSelector<Api, T>(selector)) {
return selector(apiRef);
}
return selector(apiRef.current.state);
}

// TODO v8: Rename this function to `applySelector`
function applySelectorV8<Api extends GridApiCommon, Args, T>(
function applySelector<Api extends GridApiCommon, Args, T>(
apiRef: React.MutableRefObject<Api>,
selector: Selector<Api, Args, T>,
args: Args,
Expand All @@ -38,7 +26,7 @@ function applySelectorV8<Api extends GridApiCommon, Args, T>(
if (isOutputSelector(selector)) {
return selector(apiRef, args);
}
return selector(apiRef.current.state, instanceId);
return selector(apiRef.current.state, args, instanceId);
}

const defaultCompare = Object.is;
Expand All @@ -63,55 +51,7 @@ export const argsEqual = (prev: any, curr: any) => {

const createRefs = () => ({ state: null, equals: null, selector: null, args: null }) as any;

// TODO v8: Remove this function
export const useGridSelector = <Api extends GridApiCommon, T>(
apiRef: React.MutableRefObject<Api>,
selector: ((state: Api['state']) => T) | OutputSelector<Api['state'], T>,
equals: (a: T, b: T) => boolean = defaultCompare,
) => {
if (process.env.NODE_ENV !== 'production') {
if (!apiRef.current.state) {
warnOnce([
'MUI X: `useGridSelector` has been called before the initialization of the state.',
'This hook can only be used inside the context of the grid.',
]);
}
}

const refs = useLazyRef<
{
state: T;
equals: typeof equals;
selector: typeof selector;
},
never
>(createRefs);
const didInit = refs.current.selector !== null;

const [state, setState] = React.useState<T>(
// We don't use an initialization function to avoid allocations
(didInit ? null : applySelector(apiRef, selector)) as T,
);

refs.current.state = state;
refs.current.equals = equals;
refs.current.selector = selector;

useOnMount(() => {
return apiRef.current.store.subscribe(() => {
const newState = applySelector(apiRef, refs.current.selector);
if (!refs.current.equals(refs.current.state, newState)) {
refs.current.state = newState;
setState(newState);
}
});
});

return state;
};

// TODO v8: Rename this function to `useGridSelector`
export const useGridSelectorV8 = <Api extends GridApiCommon, Args, T>(
export const useGridSelector = <Api extends GridApiCommon, Args, T>(
apiRef: React.MutableRefObject<Api>,
selector: Selector<Api, Args, T>,
args: Args = undefined as Args,
Expand Down Expand Up @@ -139,7 +79,7 @@ export const useGridSelectorV8 = <Api extends GridApiCommon, Args, T>(

const [state, setState] = React.useState<T>(
// We don't use an initialization function to avoid allocations
(didInit ? null : applySelectorV8(apiRef, selector, args, apiRef.current.instanceId)) as T,
(didInit ? null : applySelector(apiRef, selector, args, apiRef.current.instanceId)) as T,
);

refs.current.state = state;
Expand All @@ -149,7 +89,7 @@ export const useGridSelectorV8 = <Api extends GridApiCommon, Args, T>(
refs.current.args = args;

if (didInit && !argsEqual(prevArgs, args)) {
const newState = applySelectorV8(
const newState = applySelector(
apiRef,
refs.current.selector,
refs.current.args,
Expand All @@ -163,7 +103,7 @@ export const useGridSelectorV8 = <Api extends GridApiCommon, Args, T>(

useOnMount(() => {
return apiRef.current.store.subscribe(() => {
const newState = applySelectorV8(
const newState = applySelector(
apiRef,
refs.current.selector,
refs.current.args,
Expand Down
8 changes: 1 addition & 7 deletions packages/x-data-grid/src/internals/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,13 +150,7 @@ export type * from '../models/props/DataGridProps';
export type * from '../models/gridDataSource';
export { getColumnsToExport, defaultGetRowsToExport } from '../hooks/features/export/utils';
export * from '../utils/createControllablePromise';
export {
createSelector,
createSelectorV8,
createSelectorMemoized,
createSelectorMemoizedV8,
} from '../utils/createSelector';
export { useGridSelectorV8 } from '../hooks/utils/useGridSelector';
export { createSelector, createSelectorMemoized } from '../utils/createSelector';
export { gridRowGroupsToFetchSelector } from '../hooks/features/rows/gridRowsSelector';
export {
findParentElementFromClassName,
Expand Down
4 changes: 2 additions & 2 deletions packages/x-data-grid/src/models/api/gridStateApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export interface GridStatePrivateApi<State extends GridStateCommunity> {
* Updates a control state that binds the model, the onChange prop, and the grid state together.
* @param {GridControlStateItem>} controlState The [[GridControlStateItem]] to be registered.
*/
registerControlState: <E extends keyof GridControlledStateEventLookup>(
controlState: GridControlStateItem<State, E>,
registerControlState: <E extends keyof GridControlledStateEventLookup, Args>(
controlState: GridControlStateItem<State, Args, E>,
) => void;
}
3 changes: 2 additions & 1 deletion packages/x-data-grid/src/models/controlStateItem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,13 @@ import { GridStateCommunity } from './gridStateCommunity';

export interface GridControlStateItem<
State extends GridStateCommunity,
Args,
E extends keyof GridControlledStateEventLookup,
> {
stateId: string;
propModel?: GridEventLookup[E]['params'];
stateSelector:
| OutputSelector<State, GridControlledStateEventLookup[E]['params']>
| OutputSelector<State, Args, GridControlledStateEventLookup[E]['params']>
| ((state: State) => GridControlledStateEventLookup[E]['params']);
propOnChange?: (
model: GridControlledStateEventLookup[E]['params'],
Expand Down
2 changes: 1 addition & 1 deletion packages/x-data-grid/src/utils/createSelector.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ createSelector(
createSelector(
(state: GridStateCommunity) => state.columns.orderedFields,
(fields) => fields,
)({} as GridStateCommunity, { id: 1 });
)({} as GridStateCommunity, undefined, { id: 1 });

createSelector(
// @ts-expect-error Wrong state key
Expand Down
16 changes: 10 additions & 6 deletions packages/x-data-grid/src/utils/createSelector.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,23 @@ describe('createSelector', () => {
it('should warn if the instance ID is missing', () => {
const selector = createSelectorMemoized([], () => []);
const state = {} as GridStateCommunity;
// @ts-expect-error Need to test the warning
expect(() => selector(state)).toWarnDev(
'MUI X: A selector was called without passing the instance ID, which may impact the performance of the grid.',
);
expect(() => selector(state, { id: 0 })).not.toWarnDev();
expect(() => selector(state, undefined, { id: 0 })).not.toWarnDev();
});

it('should fallback to the default behavior when no cache key is provided', () => {
const selector = createSelectorMemoized([], () => []) as OutputSelector<
GridStateCommunity,
any,
any
>;
const state = {} as GridStateCommunity;
const instanceId = { id: 0 };
expect(selector(state, instanceId)).to.equal(selector(state, instanceId));
expect(selector(state, undefined, instanceId)).to.equal(
selector(state, undefined, instanceId),
);
});

it('should clear the cached value when another state is passed', () => {
Expand All @@ -33,13 +35,13 @@ describe('createSelector', () => {
);
const state1 = {} as GridStateCommunity;
const state2 = {} as GridStateCommunity;
const value1 = selector(state1, { id: 1 });
const value1 = selector(state1, undefined, { id: 1 });

// The default cache has maxSize=1, which forces a recomputation if another state is passed.
// Since the combiner function returns a new array, the references won't be the same.
selector(state2, { id: 1 });
selector(state2, undefined, { id: 1 });

const value2 = selector(state1, { id: 1 });
const value2 = selector(state1, undefined, { id: 1 });
expect(value1).not.to.equal(value2);
});
});
Expand All @@ -48,6 +50,7 @@ describe('createSelector', () => {
it('should return different selectors for different cache keys', () => {
const selector = createSelectorMemoized([], () => []) as OutputSelector<
GridStateCommunity,
any,
any
>;
const apiRef1 = {
Expand All @@ -62,6 +65,7 @@ describe('createSelector', () => {
it('should not clear the cache of one selector when another key is passed', () => {
const selector = createSelectorMemoized([], () => []) as OutputSelector<
GridStateCommunity,
any,
any
>;
const apiRef1 = {
Expand Down
Loading
Loading