mui / material-ui

Material UI: Comprehensive React component library that implements Google's Material Design. Free forever.
https://mui.com/material-ui/
MIT License
93.59k stars 32.21k forks source link

Component prop rejects my component in TypeScript #19536

Closed larsivi closed 3 years ago

larsivi commented 4 years ago

I am using the Collapse component in a TypeScript project. I am trying to set a custom trigger component for the Collapse component, using the component prop. This works if the component is a dumb one, but not if it requires props. I want a collapse trigger that is dynamically generated such that it can show a summary of the collapsed contents.

Current Behavior 😯

I get the following error

62,19): No overload matches this call.
  Overload 1 of 2, '(props: CollapseProps, context?: any): ReactElement<any, string | ((props: any) => ReactElement<any, string | ... | (new (props: any) => Component<any, any, any>)> | null) | (new (props: any) => Component<...>)> | Component<...> | null', gave the following error.
    Type 'Element' is not assignable to type '"object" | "a" | "abbr" | "address" | "area" | "article" | "aside" | "audio" | "b" | "base" | "bdi" | "bdo" | "big" | "blockquote" | "body" | "br" | "button" | "canvas" | "caption" | ... 99 more ... | undefined'.
      Type 'Element' is not assignable to type 'FunctionComponent<TransitionProps>'.
        Type 'Element' provides no match for the signature '(props: PropsWithChildren<TransitionProps>, context?: any): ReactElement<any, string | ((props: any) => ReactElement<any, string | ... | (new (props: any) => Component<any, any, any>)> | null) | (new (props: any) => Component<...>)> | null'.
  Overload 2 of 2, '(props: PropsWithChildren<CollapseProps>, context?: any): ReactElement<any, string | ((props: any) => ReactElement<any, string | ... | (new (props: any) => Component<any, any, any>)> | null) | (new (props: any) => Component<...>)> | Component<...> | null', gave the following error.
    Type 'Element' is not assignable to type '"object" | "a" | "abbr" | "address" | "area" | "article" | "aside" | "audio" | "b" | "base" | "bdi" | "bdo" | "big" | "blockquote" | "body" | "br" | "button" | "canvas" | "caption" | ... 99 more ... | undefined'.
      Type 'Element' is not assignable to type 'FunctionComponent<TransitionProps>'.

Expected Behavior 🤔

I would expect that my component works as if it was JS instead of TypeScript, or some documentation for the Collapse component stating how I should typify my component. I only found a general reference to create an issue here if I were to find such a rejection though, so here I am. Also seems to be similar to issue #17699 that was fixed.

Steps to Reproduce 🕹

Here is a codesandbox showing the issue. https://codesandbox.io/s/heuristic-lalande-plt53

Also a sandbox to show that it is not an issue in JS. https://codesandbox.io/s/material-demo-6j9ef

Steps:

1 The error is directly visible in the sandbox.

Context 🔦

I am trying to have a dynamic Collapse trigger.

Your Environment 🌎

Tech Version
Material-UI v4.9.1
TypeScript 3.8
abbasyadollahi commented 4 years ago

Seems to be an issue with more than just the Collapse component, I'm having the same problem with CardHeader. I think the problem is the type definitions are restricted to the default component used instead of being overridable like the Chip or Avatar components.

Here's a sandbox with my code showing the similar issue. As you can see, it works for Chip but doesn't work for CardHeader. https://codesandbox.io/s/quirky-yalow-9m3iq

If I were to update the type definitions for CardHeader to resemble the ones using an overridable defaultComponent, than it would work. Here's the code for that (the same thing can be done with Collapse).

import * as React from 'react';
import { TypographyProps } from '../Typography';
import { OverridableComponent, OverrideProps } from '../OverridableComponent';

export interface CardHeaderTypeMap<P = {}, D extends React.ElementType = 'div'> {
  props: P & {
    action?: React.ReactNode;
    avatar?: React.ReactNode;
    disableTypography?: boolean;
    subheader?: React.ReactNode;
    subheaderTypographyProps?: Partial<TypographyProps>;
    title?: React.ReactNode;
    titleTypographyProps?: Partial<TypographyProps>;
  }
  defaultComponent: D;
  classKey: CardHeaderClassKey;
}

declare const CardHeader: OverridableComponent<CardHeaderTypeMap>;

export type CardHeaderClassKey =
  | 'root'
  | 'avatar'
  | 'action'
  | 'content'
  | 'title'
  | 'subheader';

export type CardHeaderProps<
  D extends React.ElementType = CardHeaderTypeMap['defaultComponent'],
  P = {}
> = OverrideProps<CardHeaderTypeMap<P, D>, D>;

export default CardHeader;
abbasyadollahi commented 4 years ago

I could push out a change, looks like it'll be of the same flavor as this PR. https://github.com/mui-org/material-ui/pull/18868

oliviertassinari commented 4 years ago

@theGirrafish Thanks for the effort in #20110, we are getting closer to full coverage of the OverridableComponent types.

oliviertassinari commented 4 years ago

However, the last time I have checked, the approach isn't compatible with hosting the prop's description in TypeScript nor with the XXComponent/XXProps props. I'm not sure it scales very well.

larsivi commented 3 years ago

For what it is worth, I rewrote my uses to make my trigger component into a button, then route the clicks onto the Collapse.in property. Slightly less elegant IMO, but functionally equivalent to what I wanted. This doesn't invalidate the typescript issues when you actually want to use the component property though.

eps1lon commented 3 years ago

Thanks for the report.

codesandbox.io/s/heuristic-lalande-plt53

This is expected behavior since Collapse passes in some props that you don't handle.

TypeScript improved their error message so it may be a bit more clear now:

Type '({ foo }: { foo: any; }) => Element' is not assignable to type '"object" | "span" | "style" | "h1" | "h2" | "h3" | "h4" | "h5" | "h6" | "caption" | "button" | "a" | "abbr" | "address" | "area" | "article" | "aside" | "audio" | "b" | "base" | "bdi" | ... 98 more ... | undefined'.
  Type '({ foo }: { foo: any; }) => Element' is not assignable to type 'FunctionComponent<TransitionProps>'.
    Types of parameters '__0' and 'props' are incompatible.
      Property 'foo' is missing in type 'TransitionProps & { children?: ReactNode; }' but required in type '{ foo: any; }'.ts(2322)
Collapse.d.ts(19, 3): The expected type comes from property 'component' which is declared here on type 'IntrinsicAttributes & CollapseProps'