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

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

Open
bedrich-schindler opened this issue Sep 23, 2020 · 4 comments
Labels
new validator request Requests for a new kind of validator.

Comments

@bedrich-schindler
Copy link

bedrich-schindler commented Sep 23, 2020

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

I would definitely appreciate that!

@ljharb
Copy link
Collaborator

ljharb commented Sep 23, 2020

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

bedrich-schindler commented Sep 24, 2020

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

ljharb commented Sep 25, 2020

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.

@ljharb ljharb added the new validator request Requests for a new kind of validator. label Sep 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new validator request Requests for a new kind of validator.
Projects
None yet
Development

No branches or pull requests

3 participants