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

[RFC] Issues with slotProps overrides #14525

Open
cherniavskii opened this issue Sep 6, 2024 · 5 comments
Open

[RFC] Issues with slotProps overrides #14525

cherniavskii opened this issue Sep 6, 2024 · 5 comments
Labels
component: data grid This is the name of the generic UI component, not the React module! RFC Request For Comments v8.x

Comments

@cherniavskii
Copy link
Member

cherniavskii commented Sep 6, 2024

Context

I was working on #14424 and experienced issues with the current solution for slotProps overrides. I tried to condense the problems in the list below.

What's the problem?

  1. Custom slotProps are optional – there are no errors if they're not passed:

    declare module '@mui/x-data-grid' {
      interface FooterPropsOverrides {
        example1Prop: string;
      }
    }
    
    // ...
    
    <DataGrid
      slotProps={{
        // @ts-expect-error
        footer: {},
      }}
    />

    Demo: https://stackblitz.com/edit/react-ceq5nm?file=Demo.tsx

  2. Using module augmentation for custom slotProps affects the whole project and cannot be scoped.

    For example, if you have 2 DataGrids in your app, each using a different custom footer slot, all type overrides for that slot are merged. Currently, this is not causing issues, but if we solve (1), then custom footer props from both DataGrids will be required in slotProps.footer

    Demo: https://stackblitz.com/edit/react-ceq5nm-xssqym?file=Demo1.tsx,Demo2.tsx

  3. Overriding the type of the existing prop is not possible.

    For example, loadingOverlay has a variant: 'circular-progress' | 'linear-progress' | 'skeleton' prop. If you try to override it with variant: 'one' | 'two' it won't work, even though your custom slot implementation supports the new variant type.

    Demo: https://stackblitz.com/edit/react-ceq5nm-twuqy6

What are the requirements?

A better solution for slotProps overrides that solves the 3 problems above.

What are our options?

TBD

Proposed solution

No response

Resources and benchmarks

No response

Search keywords:

@cherniavskii cherniavskii added component: data grid This is the name of the generic UI component, not the React module! status: waiting for maintainer These issues haven't been looked at yet by a maintainer RFC Request For Comments v8.x and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Sep 6, 2024
@michelengelen
Copy link
Member

Hey @cherniavskii I did find a solution to this:

export type DistributiveOmit<T, K extends keyof any> = T extends any ? Omit<T, K> : never;

export type Overwrite<T, U> = DistributiveOmit<T, keyof U> & U;

With this you can wrap the original props in Partial and apply the overwrite to the augmented props override:

cell: Overwrite<Partial<GridCellProps>, CellPropsOverrides>;

It then applies the type properties from the CellPropsOverrides onto the slotProp and uses the correct type for it:

Screenshot 2024-09-10 at 09 25 58

One small adjustment to this:

export type GridSlotsComponentsProps = Partial<{
  [K in keyof GridSlotProps]: GridSlotProps[K];
}>;

WDYT?

@michelengelen
Copy link
Member

Here is the diff for that:

diff --git a/packages/x-data-grid/src/models/gridSlotsComponentsProps.ts b/packages/x-data-grid/src/models/gridSlotsComponentsProps.ts
index 6ea9c9f27..b99ac8698 100644
--- a/packages/x-data-grid/src/models/gridSlotsComponentsProps.ts
+++ b/packages/x-data-grid/src/models/gridSlotsComponentsProps.ts
@@ -31,6 +31,10 @@ import type { GridLoadingOverlayProps } from '../components/GridLoadingOverlay';
 import type { GridRowCountProps } from '../components/GridRowCount';
 import type { GridColumnHeaderSortIconProps } from '../components/columnHeaders/GridColumnHeaderSortIcon';

+export type DistributiveOmit<T, K extends keyof any> = T extends any ? Omit<T, K> : never;
+
+export type Overwrite<T, U> = DistributiveOmit<T, keyof U> & U;
+
 // Overrides for module augmentation
 export interface BaseCheckboxPropsOverrides {}
 export interface BaseTextFieldPropsOverrides {}
@@ -66,49 +70,60 @@ export interface SkeletonCellPropsOverrides {}
 export interface RowPropsOverrides {}

 export interface GridSlotProps {
-  baseCheckbox: CheckboxProps & BaseCheckboxPropsOverrides;
-  baseTextField: TextFieldProps & BaseTextFieldPropsOverrides;
-  baseFormControl: FormControlProps & BaseFormControlPropsOverrides;
-  baseSelect: SelectProps & BaseSelectPropsOverrides;
-  baseSwitch: SwitchProps & BaseSwitchPropsOverrides;
-  baseButton: ButtonProps & BaseButtonPropsOverrides;
-  baseIconButton: IconButtonProps & BaseIconButtonPropsOverrides;
-  basePopper: PopperProps & BasePopperPropsOverrides;
-  baseTooltip: TooltipProps & BaseTooltipPropsOverrides;
-  baseInputLabel: InputLabelProps & BaseInputLabelPropsOverrides;
-  baseInputAdornment: InputAdornmentProps & BaseInputAdornmentPropsOverrides;
-  baseSelectOption: {
-    native: boolean;
-    value: any;
-    children?: React.ReactNode;
-  } & BaseSelectOptionPropsOverrides;
-  baseChip: ChipProps & BaseChipPropsOverrides;
-  cell: GridCellProps & CellPropsOverrides;
+  baseCheckbox: Overwrite<Partial<CheckboxProps>, BaseCheckboxPropsOverrides>;
+  baseTextField: Overwrite<Partial<TextFieldProps>, BaseTextFieldPropsOverrides>;
+  baseFormControl: Overwrite<Partial<FormControlProps>, BaseFormControlPropsOverrides>;
+  baseSelect: Overwrite<Partial<SelectProps>, BaseSelectPropsOverrides>;
+  baseSwitch: Overwrite<Partial<SwitchProps>, BaseSwitchPropsOverrides>;
+  baseButton: Overwrite<Partial<ButtonProps>, BaseButtonPropsOverrides>;
+  baseIconButton: Overwrite<Partial<IconButtonProps>, BaseIconButtonPropsOverrides>;
+  basePopper: Overwrite<Partial<PopperProps>, BasePopperPropsOverrides>;
+  baseTooltip: Overwrite<Partial<TooltipProps>, BaseTooltipPropsOverrides>;
+  baseInputLabel: Overwrite<Partial<InputLabelProps>, BaseInputLabelPropsOverrides>;
+  baseInputAdornment: Overwrite<Partial<InputAdornmentProps>, BaseInputAdornmentPropsOverrides>;
+  baseSelectOption: Overwrite<
+    Partial<{
+      native: boolean;
+      value: any;
+      children?: React.ReactNode;
+    }>,
+    BaseSelectOptionPropsOverrides
+  >;
+  baseChip: Overwrite<Partial<ChipProps>, BaseChipPropsOverrides>;
+  cell: Overwrite<Partial<GridCellProps>, CellPropsOverrides>;
   columnHeaders: GridColumnHeadersProps;
-  columnHeaderFilterIconButton: ColumnHeaderFilterIconButtonProps &
-    ColumnHeaderFilterIconButtonPropsOverrides;
-  columnHeaderSortIcon: GridColumnHeaderSortIconProps & ColumnHeaderSortIconPropsOverrides;
-  columnMenu: GridColumnMenuProps & ColumnMenuPropsOverrides;
-  columnsPanel: GridColumnsPanelProps & ColumnsPanelPropsOverrides;
-  columnsManagement: GridColumnsManagementProps & ColumnsManagementPropsOverrides;
-  detailPanels: GridDetailPanelsProps & DetailPanelsPropsOverrides;
-  filterPanel: GridFilterPanelProps & FilterPanelPropsOverrides;
-  footer: GridFooterContainerProps & FooterPropsOverrides;
-  footerRowCount: GridRowCountProps & FooterRowCountOverrides;
-  loadingOverlay: GridLoadingOverlayProps & LoadingOverlayPropsOverrides;
-  noResultsOverlay: GridOverlayProps & NoResultsOverlayPropsOverrides;
-  noRowsOverlay: GridOverlayProps & NoRowsOverlayPropsOverrides;
-  pagination: Partial<TablePaginationProps> & PaginationPropsOverrides;
-  panel: GridPanelProps & PanelPropsOverrides;
-  pinnedRows: GridPinnedRowsProps & PinnedRowsPropsOverrides;
-  row: GridRowProps & RowPropsOverrides;
-  skeletonCell: GridSkeletonCellProps & SkeletonCellPropsOverrides;
-  toolbar: GridToolbarProps & ToolbarPropsOverrides;
+  columnHeaderFilterIconButton: Overwrite<
+    Partial<ColumnHeaderFilterIconButtonProps>,
+    ColumnHeaderFilterIconButtonPropsOverrides
+  >;
+  columnHeaderSortIcon: Overwrite<
+    Partial<GridColumnHeaderSortIconProps>,
+    ColumnHeaderSortIconPropsOverrides
+  >;
+  columnMenu: Overwrite<Partial<GridColumnMenuProps>, ColumnMenuPropsOverrides>;
+  columnsPanel: Overwrite<Partial<GridColumnsPanelProps>, ColumnsPanelPropsOverrides>;
+  columnsManagement: Overwrite<
+    Partial<GridColumnsManagementProps>,
+    ColumnsManagementPropsOverrides
+  >;
+  detailPanels: Overwrite<Partial<GridDetailPanelsProps>, DetailPanelsPropsOverrides>;
+  filterPanel: Overwrite<Partial<GridFilterPanelProps>, FilterPanelPropsOverrides>;
+  footer: Overwrite<Partial<GridFooterContainerProps>, FooterPropsOverrides>;
+  footerRowCount: Overwrite<Partial<GridRowCountProps>, FooterRowCountOverrides>;
+  loadingOverlay: Overwrite<Partial<GridLoadingOverlayProps>, LoadingOverlayPropsOverrides>;
+  noResultsOverlay: Overwrite<Partial<GridOverlayProps>, NoResultsOverlayPropsOverrides>;
+  noRowsOverlay: Overwrite<Partial<GridOverlayProps>, NoRowsOverlayPropsOverrides>;
+  pagination: Overwrite<Partial<TablePaginationProps>, PaginationPropsOverrides>;
+  panel: Overwrite<Partial<GridPanelProps>, PanelPropsOverrides>;
+  pinnedRows: Overwrite<Partial<GridPinnedRowsProps>, PinnedRowsPropsOverrides>;
+  row: Overwrite<Partial<GridRowProps>, RowPropsOverrides>;
+  skeletonCell: Overwrite<Partial<GridSkeletonCellProps>, SkeletonCellPropsOverrides>;
+  toolbar: Overwrite<Partial<GridToolbarProps>, ToolbarPropsOverrides>;
 }

 /**
  * Overridable components props dynamically passed to the component at rendering.
  */
 export type GridSlotsComponentsProps = Partial<{
-  [K in keyof GridSlotProps]: Partial<GridSlotProps[K]>;
+  [K in keyof GridSlotProps]: GridSlotProps[K];
 }>;

@michelengelen
Copy link
Member

Lets you overwrite the original props as well:

declare module '@mui/x-data-grid' {
  interface CellPropsOverrides {
    example1Prop?: string;
    align: 'left';
  }
}
Screenshot 2024-09-10 at 16 06 05

@cherniavskii
Copy link
Member Author

Thanks @michelengelen!
It looks like your proposal solves problems (1) and (3)!
We still have (2) unsolved, but it's already a step forward 👍🏻

@michelengelen
Copy link
Member

Thanks @michelengelen! It looks like your proposal solves problems (1) and (3)! We still have (2) unsolved, but it's already a step forward 👍🏻

Yes, that's a whole different problem in itself, so I left it out ... I am not sure if there is a solution for this. I will try and research that a bit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: data grid This is the name of the generic UI component, not the React module! RFC Request For Comments v8.x
Projects
None yet
Development

No branches or pull requests

2 participants