[datagrid] Add typing to slot props

[romgrk]

Currently all our slots are typed as React.JSXElementConstructor<any>. The downsides are:

• It's harder for us to refactor without creating bugs
• Ourselves and users don't get autocomplete & typechecking when using slots
• If we release a breaking change, it won't cause a typecheck error.

I propose that we add typing to all our slots before the v7 release.

@mui/xgrid 👍 / 👎 ?

Search keywords:

[flaviendelangle]

The main downside of typing the slots (not the slotProps) is that is quickly very complex for people to pass custom components.

Thinks like the following examples fire a TS error:

const MyCustomToolbar = ({ shouldRenderToolbar, ...other }: GridToolbarProps & { shouldRenderToolbar: boolean }) => {
  if (!shouldRenderToolbar) return null;

  return <GridToolbar {...other} />
}

const App = () => {
  const [shouldRenderToolbar, setShouldRenderToolbar] = React.useState(false);

  return (
    <React.Fragment>
      <button onClick={() => setShouldRenderToolbar(prev => !prev)}>Toggle toolbar visibility</button>
      <DataGrid slots={{ toolbar: MyCustomToolbar }} />
    </React.Fragment>
  );
}

Same idea if you want to change the DOM element at the root of a slot.

On the pickers, we are typing most of our slots, but I removed the typing from some slots like the field one because it was creating too much friction, even on our own doc examples.

On the core, as far as I know, the slots are never typed (I would have missed to exceptions).

I don't know what the best solution for the grid is, but be careful to not just type all the slots super strictly without taking the downsides into account.

[romgrk]

For the custom components/props, that's why we have the props overrides, isn't it?: https://github.com/mui/mui-x/blob/next/packages/grid/x-data-grid/src/models/gridSlotsComponentsProps.ts

So IIUC users can do something like this:

declare module '@mui/x-data-grid' {
  interface ToolbarPropsOverrides {
    something: number;
  }
}

[flaviendelangle]

Yes it's the goal of these overrides, but they come with two major limitations:

1. The override will impact the user whole codebase, you cannot have one toolbar somewhere in your app that require a prop and another toolbar that don't
2. You cannot AFAIK change / remove existing props from the typing

[romgrk]

Wouldn't it make sense to recommend to users to cast their components upon use instead?

const App = () => {
  return (
    <React.Fragment>
      <DataGrid slots={{ toolbar: MyCustomToolbar as DataGridProps['slots']['toolbar'] }} />
    </React.Fragment>
  );
}

[flaviendelangle]

It can be a solution yes, but you are loosing a lot of TS capacity if you have to do it.

Honestly, I'm not sure that typing the slots too strictly is the right approach, and I would be curious to know exactly the core team never type them.

[romgrk]

But right now we are skipping TS checking altogether because we type as any. My main concern is that right now, if we do a breaking change (which we have a few in the sticky PR), users have no way to notice other than reading carefully our migration guide. Getting safety there in exchange for having to manually cast custom slots that have additional props (narrower use-case) feels like an acceptable trade-off.

Besides, if users want convenience and opt-out of safety, they can also simply use { toolbar: CustomToolbar as any }, which isn't that much longer to write than { toolbar: CustomToolbar } and has roughly the same level of safety.

[cherniavskii]

But right now we are skipping TS checking altogether because we type as any. My main concern is that right now, if we do a breaking change (which we have a few in the sticky PR), users have no way to notice other than reading carefully our migration guide.

@romgrk
If we type the slots props and recommend casting components upon their use like suggested in #11687 (comment), would it be any different from the current situation? IIUC, TS won't complain if we remove/change the props, right?

[romgrk]

I think we should recommend doing this:

// Internal code:
type Component<T> = (props: T) => {}

type CellProps = { id: string }
type CellSlot = Component<CellProps>

type Slots = {
    cell: CellSlot,
}

// User code:
function CustomCell(props: CellProps & { custom: number }) {
    return {}
}

const slots: Slots = {
    cell: CustomCell as CellSlot,
}

[romgrk]

Another strong argument to add typings is that with our upcoming objective to use the grid without MUI bundling, we're bound to use the slots more extensively and increase the number of them. It's going to be much more safe to add new design systems if we can count on the typings to help both the implementation of adding new design systems and ensuring breaking changes don't break things silently.

[cherniavskii]

@romgrk

Another strong argument to add typings is that with our upcoming objective to use the grid without MUI bundling, we're bound to use the slots more extensively and increase the number of them.

Agree, I experienced this problem in #8067 (comment)
Your proposal looks good to me 👍🏻