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

Component prop rejects my component in TypeScript #19536

Closed
2 tasks done
larsivi opened this issue Feb 2, 2020 · 6 comments
Closed
2 tasks done

Component prop rejects my component in TypeScript #19536

larsivi opened this issue Feb 2, 2020 · 6 comments
Labels
new feature New feature or request typescript

Comments

@larsivi
Copy link

larsivi commented Feb 2, 2020

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.

  • The issue is present in the latest release.
  • I have searched the issues of this repository and believe that this is not a duplicate.

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
Copy link

abbasyadollahi commented Mar 13, 2020

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
Copy link

I could push out a change, looks like it'll be of the same flavor as this PR.
#18868

@oliviertassinari
Copy link
Member

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

@oliviertassinari oliviertassinari changed the title Collapse component prop rejects my component in TypeScript Component prop rejects my component in TypeScript Mar 20, 2020
@oliviertassinari oliviertassinari added ready to take Help wanted. Guidance available. There is a high chance the change will be accepted and removed component: Collapse The React component labels Mar 20, 2020
@oliviertassinari
Copy link
Member

oliviertassinari commented Mar 20, 2020

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.

@oliviertassinari oliviertassinari removed the ready to take Help wanted. Guidance available. There is a high chance the change will be accepted label Mar 20, 2020
@larsivi
Copy link
Author

larsivi commented Nov 20, 2020

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
Copy link
Member

eps1lon commented Jun 16, 2021

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'

@eps1lon eps1lon closed this as completed Jun 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature New feature or request typescript
Projects
None yet
Development

No branches or pull requests

4 participants