react-bootstrap / react-overlays

Utilities for creating robust overlay components
https://react-bootstrap.github.io/react-overlays
MIT License
898 stars 223 forks source link

Use render prop instead of injecting props into children #243

Open felixfbecker opened 6 years ago

felixfbecker commented 6 years ago

Injecting props implicitly into children is confusing because it is not clear from reading the code where they come from. More importantly, it is impossible to type check with TypeScript or Flow - if the component requires the props (and uses them), but the reference to the component doesn't pass them explicitly, TS will error that a Prop is missing, even though they are passed implicitly. The type cannot be checked either.

The better alternative would be to use render props:

<Overlay renderContent={({ style }) => <Tooltip style={style} />} />
jquense commented 6 years ago

I'm not sure the render prop gains anything here over using a component to control how props are passed through.

const MyTooltip = ({ style }) => <Tooltip style={style} />

...

<Overlay overlay={<MyTooltip />} />
felixfbecker commented 6 years ago

You are right that you can just as well pass a component to the render prop, after all in my example ({ style }) => <Tooltip style={style} /> is a React functional component.

But in your second line, it needs to be:

<Overlay overlay={MyTooltip} />

Otherwise there would be no way to pass the style prop.

jquense commented 6 years ago

Otherwise there would be no way to pass the style prop.

I'm not sure what you mean there, the style prop will be injected by Overlay.

felixfbecker commented 6 years ago

You gave this example

<Overlay overlay={<MyTooltip />} />

In that example, React will first instantiate MyTooltip (passing it no props {}), then pass that instance to Overlay as the overlay prop. TypeScript will give you a compile error, because MyTooltip has declared that it needs the style prop to be passed, but it isn't passed at the point where it is instantiated. Same issue as in the OP.

If however, you pass the reference to the component or a function (not the instance), like this:

<Overlay overlay={MyTooltip} />
// or
<Overlay overlay={({ style }) => <MyTooltip style={style} />} />

the type safety is guaranteed.

This pattern is very popular in almost every React library that allows customization of how parts of a component are rendered.