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

[Accordion] Accepts "component" prop, but TS says it shouldn't #36940

Closed
2 tasks done
zachintosh opened this issue Apr 19, 2023 · 4 comments · Fixed by #37111
Closed
2 tasks done

[Accordion] Accepts "component" prop, but TS says it shouldn't #36940

zachintosh opened this issue Apr 19, 2023 · 4 comments · Fixed by #37111
Assignees
Labels
component: accordion This is the name of the generic UI component, not the React module! typescript

Comments

@zachintosh
Copy link

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Steps to reproduce 🕹

Link to live example:

https://codesandbox.io/s/brave-night-r23fp0?file=/src/App.tsx

Steps:

  1. Open App.tsx
  2. See the TS error on the usage of Accordion

Current behavior 😯

Applying the "component" prop on Accordion causes Typescript to throw an error, but it works as expected. The 'component' prop element is used.

Expected behavior 🤔

Either the prop types for Accordion need to be fixed or update Accordion to reject component.

Context 🔦

This is comes from an oddly specific situation where we need to make Accordion also a Draggable (from another library). We can work around it, so it isn't preventing us from anything critical, but noticed this bug along the way.

Not critical in any way that we get this fixed. Just through we'd point it out in case it's an inconsistency that needs to be looked at.

Your environment 🌎

npx @mui/envinfo

Using Brave browser (Chromium) and VSCode

  System:
    OS: macOS 13.3.1
  Binaries:
    Node: 16.19.0 - ~/Library/Caches/fnm_multishells/2045_1681829681683/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 8.19.3 - ~/Library/Caches/fnm_multishells/2045_1681829681683/bin/npm
  Browsers:
    Chrome: Not Found
    Edge: Not Found
    Firefox: Not Found
    Safari: 16.4
  npmPackages:
    @emotion/react: ^11.5.0 => 11.10.6
    @emotion/styled: ^11.3.0 => 11.10.6
    @mui/base:  5.0.0-alpha.122
    @mui/core-downloads-tracker:  5.11.15
    @mui/icons-material: ^5.11.11 => 5.11.11
    @mui/lab: ^5.0.0-alpha.71 => 5.0.0-alpha.124
    @mui/material: ^5.11.14 => 5.11.15
    @mui/private-theming:  5.11.13
    @mui/styled-engine:  5.11.11
    @mui/styles: ^5.0.1 => 5.11.13
    @mui/system:  5.11.14
    @mui/types:  7.2.3
    @mui/utils:  5.11.13
    @mui/x-data-grid:  5.17.26
    @mui/x-data-grid-pro: ^5.0.0-beta.7 => 5.17.26
    @mui/x-date-pickers: ^5.0.0-alpha.7 => 5.0.20
    @mui/x-license-pro:  5.17.12
    @types/react:  18.0.29
    react: ^18.2.0 => 18.2.0
    react-dom: ^18.2.0 => 18.2.0
    typescript: ^4.9.4 => 4.9.5
@zachintosh zachintosh added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Apr 19, 2023
@mj12albert mj12albert added component: accordion This is the name of the generic UI component, not the React module! and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Apr 20, 2023
@mj12albert mj12albert self-assigned this Apr 20, 2023
@mj12albert
Copy link
Member

@zachintosh Thanks for reporting this ~

export interface AccordionProps extends StandardProps<PaperProps, 'onChange'> { ... }

This isn't working.

It's also affecting other components that's using (Internal)StandardProps e.g. Alert: https://codesandbox.io/s/https-github-com-mui-material-ui-issues-36940-hbv4uy?file=/src/App.tsx

@1banni
Copy link

1banni commented Apr 24, 2023

Would it make sense to add it to PaperTypeMap?

  props: P & {
    /**
     * The content of the component.
     */
    children?: React.ReactNode;
    /**
     * Override or extend the styles applied to the component.
     */
    classes?: Partial<PaperClasses>;
    /**
     * Pass a custom React component or string representing
     * an HTML element (e.g., 'div' or 'section')
     */
    component?: React.ElementType;

or InternalStandard Props?

export type InternalStandardProps<C, Removals extends keyof C = never> = DistributiveOmit<
  C,
  'classes' | Removals
> &
  // each component declares it's classes in a separate interface for proper JSDoc
  StyledComponentProps<never> & {
    ref?: C extends { ref?: infer RefType } ? RefType : React.Ref<unknown>;
    // TODO: Remove implicit props. Up to each component.
    className?: string;
    style?: React.CSSProperties;
    component?: React.ElementType;
  };

@sai6855 sai6855 assigned sai6855 and unassigned mj12albert Apr 30, 2023
@sai6855
Copy link
Contributor

sai6855 commented Apr 30, 2023

@mj12albert are you working on this?

@mj12albert
Copy link
Member

@sai6855 Please go ahead ~

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: accordion This is the name of the generic UI component, not the React module! typescript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants