mui / material-ui

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

[material-ui] Incorrect React component prop `children` types for components like `AvatarGroup` #38533

Open jaydenseric opened 1 year ago

jaydenseric commented 1 year ago

Duplicates

Latest version

Steps to reproduce 🕹

Many of the Material UI React components that expect particular React elements in the prop children currently have an incorrect TypeScript type for the prop of React.ReactNode.

In demo.tsx:

import Avatar from "@mui/material/Avatar";
import AvatarGroup from "@mui/material/AvatarGroup";

function Demo() {
  return (
    <AvatarGroup>
      <>
        <Avatar>AA</Avatar>
        <Avatar>BB</Avatar>
      </>
    </AvatarGroup>
  );
}

Current behavior 😯

There will not be a TypeScript (or prop-types) error about using a React Fragment in the AvatarGroup prop children, but if you render the Demo component this will be logged as an error in the browser console:

MUI: The AvatarGroup component doesn't accept a Fragment as a child.
Consider providing an array instead.
Screenshot 2023-08-18 at 10 39 08 am

Expected behavior 🤔

For the current AvatarGroup API that intentionally forbids using a React Fragment in the prop children, there should be a TypeScript error that a React Fragment was used in the AvatarGroup prop children, and there should also be a prop-types warning in the browser console about the value used.

Ideally the AvatarGroup API (and the API for other components with similar expectations about React Elements in children) would be changed to allow using a React.ReactNode type value in the prop children, including a React Fragment. This could be achieved by implementing something like react-keyed-flatten-children to convert children into a flattened and keyed array, accounting for React Fragments.

Context 🔦

Here is an example where AvatarGroup has the wrong TypeScript type for the React component prop children, resulting also in the React component propTypes being incorrect…

Source for the type:

https://github.com/mui/material-ui/blob/77704301720b15371817da7c2d7a3275d3233809/packages/mui-material/src/AvatarGroup/AvatarGroup.d.ts#L12-L15

Published type:

https://unpkg.com/browse/@mui/material@5.14.5/AvatarGroup/AvatarGroup.d.ts

Resulting incorrect generated propTypes:

https://github.com/mui/material-ui/blob/77704301720b15371817da7c2d7a3275d3233809/packages/mui-material/src/AvatarGroup/AvatarGroup.js#L165-L168

Incorrect API docs:

https://mui.com/material-ui/api/avatar-group/#AvatarGroup-prop-children

Screenshot 2023-08-18 at 10 24 46 am

Some searches to try to help find other React components with an incorrect prop type for children:

Note that the current limitation of the prop children not accepting a React component for React components like AvatarGroup is particularly annoying when writing Storybook stories, when you want to provide the children via the story config args.children. For example, in AvatarGroup.stories.tsx:

import Avatar from "@mui/material/Avatar";
import AvatarGroup from "@mui/material/AvatarGroup";
import type { Meta, StoryObj } from "@storybook/react";

const meta = {
  component: AvatarGroup,
} satisfies Meta<typeof AvatarGroup>;

export default meta;

type Story = StoryObj<typeof meta>;

export const ChildrenFragment = {
  args: {
    children: (
      <>
        <Avatar>AA</Avatar>
        <Avatar>BB</Avatar>
      </>
    ),
  },
} satisfies Story;

export const ChildrenArray = {
  args: {
    children: [<Avatar>AA</Avatar>, <Avatar>BB</Avatar>],
  },
} satisfies Story;

The first story ChildrenFragment at render results in this console error:

MUI: The AvatarGroup component doesn't accept a Fragment as a child.
Consider providing an array instead.

While the second story ChildrenArray (which is what a user might attempt after following the instructions of the first console error to provide an array instead) at render results in this console error:

wrapComponent.tsx:33 Warning: Each child in a list should have a unique "key" prop.

Check the render method of `AvatarGroup`. See https://reactjs.org/link/warning-keys for more information.
    at Avatar
    at AvatarGroup
    at …

The Material UI component docs for the AvatarGroup prop children didn't give any indication there would be render issues for stories written like this, there was no build time type safety, and it's not clear to a user after manually trying to render the components and reading the console errors the best way to resolve it.

Your environment 🌎

npx @mui/envinfo ``` System: OS: macOS 13.5 Binaries: Node: 20.5.0 - ~/.volta/tools/image/node/20.5.0/bin/node Yarn: 1.22.19 - ~/.volta/tools/image/yarn/1.22.19/bin/yarn npm: 9.8.0 - ~/.volta/tools/image/node/20.5.0/bin/npm Browsers: Chrome: 116.0.5845.96 Edge: 115.0.1901.203 Safari: 16.6 npmPackages: @emotion/react: 11.11.1 @emotion/styled: ^11.11.0 => 11.11.0 @mui/base: 5.0.0-beta.11 @mui/core-downloads-tracker: 5.14.5 @mui/icons-material: ^5.14.3 => 5.14.3 @mui/material: ^5.14.5 => 5.14.5 @mui/private-theming: 5.14.5 @mui/styled-engine: 5.13.2 @mui/system: 5.14.5 @mui/types: 7.2.4 @mui/utils: 5.14.5 @types/react: ^18.2.20 => 18.2.20 react: ^18.2.0 => 18.2.0 react-dom: ^18.2.0 => 18.2.0 typescript: ^5.1.6 => 5.1.6 ```
samuelsycamore commented 1 year ago

Thanks for this detailed report @jaydenseric! It sounds like the best solution for now would be to document this use case to let others know about the potential pitfalls.