React: Feature request: renderTypes

[ljharb]

Do you want to request a feature or report a bug?
Request a feature

Per some discussion today with @tomocchino and @thejameskyle, I'd like a non-Flow mechanism to annotate what type(s) of elements a component expects to render.

Here's some examples, with Flow types for comparison (that I realize may not be currently checked in Flow, yet):

function Foo({ yes }){
  return yes ? <Bar /> : <div />;
}
Foo.renderTypes = [Bar, 'div'];


class Bar extends React.Component {
  static renderTypes = [Button];

  render() {
    return <Button />;
  }
}

function Foo({ yes }): React.Element<Bar | 'div'> {
  return yes ? <Bar /> : <div />;
}

class Bar extends React.Component {
  render(): React.Element<Button> {
    return <Button />;
  }
}

Inside @Airbnb, we have lots of use cases where we have container components in a separate package - say, a <ButtonRow>, and we have intentionally restrictive propTypes on its children prop, to only allow a Button (also in the same package). However, in an app that consumes this component library package, a dev may want to create a <SpecialProductButton /> that in turn renders a <Button> - however, they're unable to pass it into ButtonRow (our propType warnings fail tests), even though conceptually it should be permitted.

Having .renderTypes would allow us to widen our children propType to allow for either a <Button>, or anything that renders a <Button>, which helps us maintain separation of concerns (the package doesn't have to know about <SpecialProductButton> to accept it) as well as maintain strictness (the package doesn't have to allow any wacky element inside <ButtonRow>).

I imagine the implementation to be:

1. when render() is called or an SFC is invoked, (in async rendering, it'd be when the component resolves, i suppose)
2. in development only and if .renderTypes exists on the component
3. evaluate the equivalent of elementType(...Component.renderTypes)({ children: renderedValue }, 'children', ...),
4. just like propTypes, log the error if one is returned

(cc @spicyj)

[lencioni]

If I understand this correctly, you could do most of this (i.e. annotating components with .renderTypes and checking those in the restrictive propTypes checks) now, right? It sounds like this proposal is to add a runtime check to React in dev to ensure that what the component renders matches the renderTypes property. Is that accurate?

Also, what might .renderTypes look like after https://github.com/facebook/react/issues/2127 is implemented and components can render fragments?

[ljharb]

Yes, exactly - without React enforcing it, it's just adding an arbitrary convention, which imo is potentially very dangerous for the ecosystem.

I'm not sure what it would look like with fragmented rendering; but I'd assume that fragments would be able to optionally specify renderTypes as well if they wanted - there's nothing inherent about the feature as proposed that would require a single root node. In other words, if something rendered <div /><Button /><span />, renderTypes of ['div', Button, 'span'] would pass on it.

[sophiebits]

cc @sebmarkbage

[sebmarkbage]

I really want this for our Flow support since Flow can infer it all the way down and ensure. I haven't considered this for PropTypes.

We might be able to do just what you want here but, if not, coroutines in Fiber does let you implement this kind of thing in user space and is the wider solution for this kind of parent/child relationships with indirections.

[ljharb]

It'd be very valuable to use to be able to use it pre-Fiber, so hopefully this is a feature that can be added easily :-)

[sebmarkbage]

We most likely won't back port this (or most other new features) to React 15. Get on the Fiber train. :)
(We should chat about AirBnB's React 16 adoption path, btw.)

[ljharb]

How's server rendering of Fiber coming? :-p

[sophiebits]

It's landed.

[sebmarkbage]

Passes all the tests so it must be done. :)

[ljharb]

Regardless tho, it's also pretty disappointing that prior to v16 being released, a new feature for 15 wouldn't be considered. Would you be willing to accept it if I wrote up the PR?

[sebmarkbage]

We're getting started on the release work for 16 now. We'd have to switch focus from that to do another 15 release (with everything that entails to try to not break the ecosystem). It's possible we could do that if you think it's worth while but I'm not even sure how to correctly implement this in 15. Would be much easier in Fiber actually.

The reason is that in 15 we don't always have the ability for the parent to see all possible children. A deep setState on SpecialProductButton in your example could switch from rendering <Button /> to rendering <InvalidComponent /> and there would be no way to enforce that efficiently.

[ljharb]

It'd be enforcing what it actually returns, not what it theoretically could return - so you wouldn't get a renderType warning on InvalidComponent until it starts actually rendering it.

[gaearon]

Releasing 15.5 and 15.6 took several weeks of effort from several people. We are not in a good position to add any more features to 15.x at this point. The branch has diverged way too much, and it’s causing much duplicated effort. We also don't have the usual stability guarantees for it anymore because we don't use 15.x in production.

[bvaughn]

Chiming in kind of late here but did you consider something like this?

const Test = props => <div>Example</div>;

Test.propTypes = {
  prop: function(props, propName, componentName) {
    if (!recursiveTypeCheck(props[propName], 'strong')) {
      throw Error('Nooo!');
    }
  }
};

function recursiveTypeCheck(element, type) {
  if (element.type === type) {
    return true;
  } else {
    return React.Children.toArray(element.props.children)
      .some(child => recursiveTypeCheck(child, type));
  }
}

Edit Disregard! I believe the missing case is for a class/functional component that renders type. 😄

[flarnie]

Hi @ljharb - the React team talked about this issue and we came up with some concerns.
tldr: Right now we are thinking it best that we not implement this in React.

• Just because the return type is the same, doesn't mean they accept the same props or interact with CSS or other environmental things the same. This wouldn't give enough type safety to address the cases we thought of.
• Additionally, we moved prop-types out of React Core and would generally prefer other type checking to be outside of React.

*Here's some more detail about what we considered: *
We tried to get a sense of what high level problem this is meant to solve. It seems like this feature would help validate that components which a <Parent> gets passed in via props have certain behavior or a similar API, based on the type of component they render.
Though the proposed feature would work with some cases, we also thought of some common cases that have pitfalls. A common pattern we have seen is that <Parent> takes <Child> as a prop, then clones it and passes in some propX. Even if we verify that <FancyChild> returns a <ChildComponent>, we can't guarantee that it accepts propX and passes it to <Child>.
We also have seen cases where <MyButton> and <FancyButton> may both return <button>, and may even accept the same props, but their CSS or other implementation details are not compatible, such that we could not assume they would both "just work" as children in <ButtonGroup>.

We also considered the idea that this could be implemented outside of React - Sebastian commented earlier that

coroutines in Fiber does let you implement this kind of thing in user space and is the wider solution for this kind of parent/child relationships with indirections.

It could be that, as you said,

without React enforcing it, it's just adding an arbitrary convention, which imo is potentially very dangerous for the ecosystem.

That said, at this point we have moved prop-types out of React core and into a separate module, and it seems like it might be similarly ok to have a module outside of React to address this concern.

Hope this is helpful and please let me know how this sounds and if there is anything we can clarify. Thanks also for bringing this issue to my attention, so I could bring it up again with the team.

[ljharb]

Thanks for the response!

While I think a module outside of React is definitely a better way to go about it; there's still dev-only hooks into prop-types inside React, and for this feature there'd need to be similar dev-only hooks into render-types, eg - I'd be happy to make a separate package if React were willing to add those hooks.

Even if we verify that <FancyChild> returns a <ChildComponent>, we can't guarantee that it accepts propX and passes it to <Child>.

I'm not sure why - FancyChild is a known implementation.

[sophiebits]

FancyChild is a known implementation.

To clarify (this was my objection): you might have a Button component that takes buttonStyle. Maybe you have a ButtonGroup that does React.cloneElement(child, {buttonStyle: 'grouped'}). That seems to me like the most common use case in practice for this feature today.

If you do pass a FancyButton instead (and FancyButton renders to Button), ButtonGroup will clone the FancyButton element and add buttonStyle prop to FancyButton instead. But FancyButton doesn't necessarily take buttonStyle; it could take a totally different set of props.

(If typechecks before cloneElement aren't the primary value you see, I'm curious what you see as the value proposition for enforcing this.)

[ljharb]

We tend to avoid cloneElement as much as we can; the main goal here is, for example, to have a <ButtonGroup> that can only take <Button>s, and we want ButtonGroup to only allow Buttons as children, but we don't want to have to teach ButtonGroup that FancyButton exists.

So, FancyButton.renderTypes = [Button] can allow for an explicit claim by FancyButton that it renders a Button, and ButtonGroup's propType can say "i want children that are either a Button, or that render a Button".

[sophiebits]

But so why does ButtonGroup care if FancyButton returns a Button? Having that guarantee allows it to do what that it can’t otherwise?

[gaearon]

The only application I can easily see here is helping enforce CSS constraints. e.g. if only Button is .Button, then we can kinda enforce that .ButtonGroup will contain .Buttons as immediate children by doing this.

I don’t yet see where else that would be useful since the rendered child is opaque and you can’t interface with it directly.

[ljharb]

@sophiebits @gaearon It's a design consideration. We don't want devs to be able to freely put whatever they want, wherever they want. By restricting ButtonGroup to only have Buttons, then devs can't naively stick a dropdown menu or something in a ButtonGroup.

A perhaps more common pattern for us is: we have a propType called "textlike". It allows a string, or a <T> component (internal translation component), or a <Text> component (from our internal design library). However, there's lots of wrapper components that might render their own <Text> with custom hardcoded props (let's say, a function Heading({ children }) { return <Text large>{children}</Text>; }), and we'd want a <Heading> to be usable wherever <Text> is allowed, without having to embed knowledge of Heading inside the textlike propType.

[sebmarkbage]

It's subtyping vs nominal typing. Sometimes it makes sense to add some restrictions that won't technically fail but will organizationally fail.

[sophiebits]

By restricting ButtonGroup to only have Buttons, then devs can't naively stick a dropdown menu or something in a ButtonGroup.

Yes, but what bad happens if you do? :) I'm fine if the answer is "nothing, this is primarily for documentation enforced by code", just want to make sure I'm not missing anything.

wherever <Text> is allowed

If I'm authoring a component, when/why would I want to restrict my children to allow all kinds of Text but no other components? This seems further from the CSS argument even since a Heading likely has a very different appearance from any other text.

[ljharb]

The bad thing is "the interface is inconsistent, and designers (and without realizing it, users) are sad".

It's got nothing to do with CSS; we want our entire codebase as restricted as possible, and we heavily use custom children propTypes to do it already. If non-text is allowed, people (devs/engs/PMs) will put only icons there (which isn't accessible), or will abuse existing styling to use containers for layout (which often breaks on various responsive breakpoints), or will employ any of a multitude of subpar UX patterns without realizing it. We want nothing to be permitted except known-good patterns, and renderTypes (or anything that provides similar functionality) will help close some of the few remaining loopholes/awkward surfaces in this approach.

[gaearon]

I’m a bit worried that if we add this, it will turn into a cargo cult. People will be adding renderTypes “just in case” because they can, not because they understand the initial use case. It is a bit awkward that the API is about decorating the child components, but the actual use case is about validating parent-child relationship. This is not obvious from looking at the API.

[ljharb]

That's a fair criticism; this is just the solution that made sense to me. If there's another way to meet the use case that would be more obvious/intuitive, I'm all for it!

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution.

[ljharb]

Bump.

[jamiebuilds]

Not suggesting that this API needs to be in React, but what about flipping the API around to validate it from the parent?

function isButton(element) {
  return true || false
}

function ButtonGroup({ children }) {
  return (
    <div className="btn-group">
      <Assert is={isButton}>{children}</Assert>
    </div>
  )
}

[stale[bot]]

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

[ljharb]

bump

[stale[bot]]

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

[ljharb]

eternal bump

[sophiebits]

I’m going to close this.

It seems that the web dev industry has settled on TypeScript for people who want type checking in their code. Static type checks (as I’m sure you know) have the advantage that they detect problems at build time even if the faulty code isn’t exercised during development.

At this point I see it as much more likely that in the future React removes propTypes support rather than adding renderTypes. You can also write a custom linter if this is a concern specifically for your employer. It’s not a common enough request that it makes sense to add to React now.

[ljharb]

Fair enough, but that’s unfortunate, since typescript is both unsound and not a superset of JS, and can’t ever approach the validation power propTypes offer.

The benefit of this proposal is that it’s a convention for the (not small) part of the community that still uses propTypes to organize around - the goal wasn’t just for a convention inside a company, it was for the broader ecosystem as well - something only React can define.