facebook / prop-types

Runtime type checking for React props and similar objects
MIT License
4.48k stars 356 forks source link

Add support of `instanceOf` for functional components or create new `componentOf` check #327

Open bedrich-schindler opened 4 years ago

bedrich-schindler commented 4 years ago

Our team is missing support of instanceOf(type) for functional components. Motivation is that you can have parental component (e.g. Toolbar) that can only accept several components (e.g. ToolbarGroup and ToolbarItem). It is possible to check prop types when those components are implemented as classes, but not when they are implemented as function.

I have discovered that both class and functional components have its name stored in one variable:

// Toolbar.jsx
React.Children.forEach(children, (a) => {
    console.log(a.type.name === 'ToolbarItem' || a.type.name === 'ToolbarGroup');
});

It means that it should be possible to extend existing instanceOf(type) check to work with functional components or implement new one. I would prefer to implement new one because it would use different mechanism of detecting its name. It could be called for example componentOf - or you can suggest another name.

I can create pull request with this issue, but the question is if community and Facebook team want this new check.

adamkudrna commented 4 years ago

I would definitely appreciate that!

ljharb commented 4 years ago

It sounds like what you're asking for is a component propType, since function components are already instanceOf(Function). PropTypes.elementType may be helpful here.

However, a.type.name works because a is a React Element, not a component - and PropTypes.element already covers those.

Can you elaborate more on what exactly you're having trouble propTyping?

bedrich-schindler commented 4 years ago

@ljharb I will show it on an example.

Let have three components Toolbar, ToolbarItem, and ToolbarGroup. Toolbar is component that accepts only ToolbarItem and ToolbarGroup or array of those components. Simplified example:

export  const ToolbarGroup = (props) => {
  const { children } = props;

  return (
    <div className={styles.group}>
      {children}
    </div>
  );
};
export  const ToolbarItem = (props) => {
  const { children } = props;

  return (
    <div className={styles.item}>
      {children}
    </div>
  );
};
const Toolbar = (props) => {
  const { children } = props;

  return (
    <div className={styles.toolbar}>
      {children}
    </div>
  );
};

Toolbar.propTypes = {
  children: /* what check to use here? */,
};

export default Toolbar;

Example of usage of those components in valid state:

<Toolbar>
  <ToolbarItem>
    <button>...</button>
  </ToolbarItem>
  <ToolbarItem>
    <button>...</button>
  </ToolbarItem>
</Toolbar>

Example of usage of those components in invalid state because Toolbar cannot contain button:

<Toolbar>
  <button>...</button>
  <button>...</button>
</Toolbar>

The problem is that we want to be able to check whetherToolbar contains only ToolbarGroup, ToolbarItem or array of those two components. We did not found way how to check it using prop types. If there is any way, I would be delighted if you are able to show the correct prop types definition for this simple example.

If it is not possible yet, I would like to implement new check (e.g. componentOf - or whatever the name) that would be able to check it. I expect that it could look like this:

Toolbar.propTypes = {
  children: PropTypes.oneOfType([
    PropTypes.componentOf(ToolbarItem),
    PropTypes.componentOf(ToolbarGroup),
    PropTypes.arrayOf(PropTypes.oneOfType([
      PropTypes.componentOf(ToolbarItem),
      PropTypes.componentOf(ToolbarGroup),
    ])),
  ]).isRequired,
};

I believe that it can be implemented via <child>.type.name, it contains name of both class and functional component, see:

image

This new check would allow developers to restrict which children can component accept by their names. This is pretty simple example but there can be complicated hierarchies of components.

I am calling it componentOf but maybe there is better name for it.

Thank you @ljharb for your time. I appreciate that and I hope I have described it better.

ljharb commented 4 years ago

Since in this case, you're accepting elements and not components, you'd want to use elementType from https://npmjs.com/airbnb-prop-types. There's nothing in this library that offers that at the moment.