[typescript] Better type annotations for unstyled components and componentsProps props

[michaldudak]

• I have searched the issues of this repository and believe that this is not a duplicate.

Summary 💡

Improve developer experience by providing type information in componentsProps when a component/element is passed in the respective components field.

Once a developer populates the components.X field, they will see intellisense hints in componentsProps.x with all the props available to pass to the X component.

Examples 🌈

Preliminary work has been done in: michaldudak@a9f37ba

The problem with this solution is that it causes PropTypes to be generated incorrectly. For components fields, the expected prop type would be PropTypes.elementType, but a union of all intrinsic element names and PropTypes.func is generated. This is problematic when a class based component, or a one created with React.forwardRef is passed in.

Motivation 🔦

Having detailed hints in intellisense and the ability to check types before runtime will lead to increased developer happiness and productivity. As an application developer myself, I value well-crafted type definitions that help during development.

[oliviertassinari]

A reproduction related to this issue: https://codesandbox.io/s/basicalerts-material-demo-forked-pqs4e?file=/demo.tsx.

[michaldudak]

We discussed the topic with @eps1lon and he pointed out that in some cases this may not work as expected and can create more problems then it can solve. I'm therefore abandoning the effort and will leave componentsProps untyped.

[oliviertassinari]

@michaldudak OK sounds great.

Did you check the codebase to make sure all the components follow the same approach? Bases on #26810 (comment), it's not clear to me that we do. I suspect we need a follow up

Or maybe even better. What's the current standard/recommendation regarding this problem? So that @mui-org/x can also follow the convention.

[michaldudak]

The SliderUnstyled mentioned in #26810 (comment) as well as other unstyled components (except for the Switch) use (slighly) typed componentsProps. They expect just as and styleProps fields. Passing in other, possibly valid, props will cause errors unless module augmentation is used. I don't think this leads to a good developer experience. Since inferring props' types based on components' types is not an option anymore, I'd stick to Record<string, any>.

[oliviertassinari]

@michaldudak I had a look at what we have in the codebase, it seems that we could normalize:

components

It seems to be about React.JSXElementConstructor vs. React.ElementType.

• Switch

https://github.com/mui-org/material-ui/blob/8a4b8952077b64e90ac9d23c116a45aed672db9c/packages/material-ui-unstyled/src/SwitchUnstyled/SwitchUnstyled.tsx#L19-L28

• Slider

https://github.com/mui-org/material-ui/blob/8a4b8952077b64e90ac9d23c116a45aed672db9c/packages/material-ui-unstyled/src/SliderUnstyled/SliderUnstyled.d.ts#L36-L49

• pickers

https://github.com/mui-org/material-ui/blob/8a4b8952077b64e90ac9d23c116a45aed672db9c/packages/material-ui-lab/src/ClockPicker/ClockPicker.tsx#L92-L101

• DataGrid

/**
 * The set of overridable components used in the grid.
 */
components: {
  /**
   * The custom Checkbox component used in the grid.
   */
  Checkbox: React.ElementType;
  /**
   * Column menu component rendered by clicking on the 3 dots "kebab" icon in column headers.
   */
  ColumnMenu: React.JSXElementConstructor<any>;

componentsProps

It seems to be about any vs. {} vs. Partial< vs. { as, styleProps } .

• Switch

https://github.com/mui-org/material-ui/blob/8a4b8952077b64e90ac9d23c116a45aed672db9c/packages/material-ui-unstyled/src/SwitchUnstyled/SwitchUnstyled.tsx#L30-L39

• Slider

https://github.com/mui-org/material-ui/blob/8a4b8952077b64e90ac9d23c116a45aed672db9c/packages/material-ui-unstyled/src/SliderUnstyled/SliderUnstyled.d.ts#L50-L94

• pickers

https://github.com/mui-org/material-ui/blob/8a4b8952077b64e90ac9d23c116a45aed672db9c/packages/material-ui-lab/src/ClockPicker/ClockPicker.tsx#L103-L109

• Autocomplete

https://github.com/mui-org/material-ui/blob/8a4b8952077b64e90ac9d23c116a45aed672db9c/packages/material-ui/src/Autocomplete/Autocomplete.d.ts#L97-L103

• DataGrid

/**
 * Overrideable components props dynamically passed to the component at rendering.
 */
componentsProps?: {
  checkbox?: any;
  columnMenu?: any;
  errorOverlay?: any;
  footer?: any;
  header?: any;
  toolbar?: any;
  preferencesPanel?: any;
  loadingOverlay?: any;
  noResultsOverlay?: any;
  noRowsOverlay?: any;
  pagination?: any;
  filterPanel?: any;
  panel?: any;
  columnsPanel?: any;
}

[michaldudak]

AFAIK you can't assign HTML element names to JSXElementConstructor, so ElementType seems to be a better option.

As for componentsProps, we could at least constrain it to be an object with string keys, so Record<string, any> is more appropriate. any is too loose and {} won't allow property access without casting. Partial<...> suffers from the same issue as ``doesn't help either as `{ as, styleProps }``` - adding another field will cause an error.

[oliviertassinari]

AFAIK you can't assign HTML element names to JSXElementConstructor, so ElementType seems to be a better option.

@michaldudak True, so I guess it depends on either we envision the support for a React host element or not for the specific slot? I mean, as soon as we provide custom props, it's KO for using JSXElementConstructor.

---

Record<string, any> 👍

[oliviertassinari]

I'm reopening so we can use this issue as a pretext to normalize.

[michaldudak]

True, so I guess it depends on either we envision the support for a React host element or not for the specific slot? I mean, as soon as we provide custom props, it's KO for using JSXElementConstructor.

If there are cases where we require a custom component we could use ComponentType or JSXElementConstructor. Otherwise ElementType should work.

[michaldudak]

@mnajdova You were using {as, styleProps} in the first unstyled components. Do you have an opinion about using Record<string, any> instead, as proposed here?

[mnajdova]

Yeah, let's go with it. Developers can anyway augment the props interface if they want to make it stricter.