Resolve inconsistencies when forwarding components

[jonkoops]

Describe the issue. What is the expected and unexpected behavior?

Sometimes as a user you want to be able to pass in your custom component to render instead of the default element. However the API around this varies wildly per component. For example, these are some components we use in our application that need React Router's Link component:

Button

{/* @ts-ignore */}
<Button component={Link} to={toNewClientScope({ realm })}>
  {t("createClientScope")}
</Button>

DropdownItem

<DropdownItem
  component={<Link to={toDashboard({ realm })}>{t("realmInfo")}</Link>}
/>

BreadcrumbItem

<BreadcrumbItem
  render={(props) => (
    <Link {...props} to={toRealmSettings({ realm, tab: "keys" })}>
      {t("keys")}
    </Link>
  )}
/>

As you can see there are already three different ways of forwarding a component to render, and some of them are not able to be written in a type-safe manner.

The expected behavior here would be that this is a single and consistent API that is completely type-safe. For example, Styled Components has a as prop which is made type-safe by the definitions and allows the props to be forwarded and type-checked as well:

import styled from 'styled-components'

const Button = styled.button`
  background-color: hotpink;
`

<Button component={Link} to={toNewClientScope({ realm })}>
  {t("createClientScope")}
</Button>

[tlabaj]

hi @jonkoops, yes the API is not consistent. In some cases support for React Router Link was added after the component was developed when consumers requested support. We want to be cognizant of adding support without breaking the existing API. In order to make the API's consistent across all components, we would need to introduce a breaking change.

[jonkoops]

Yeah for sure this is a hard one. Perhaps a generic functionality could be introduced under a new prop like as and the others could be marked as deprecated.

In this context a deprecation would simply nudge the user to use the new solution using a warning, without immediately removing the functionality right in that moment.

[nicolethoen]

@mcarrano could we add this spike to a future milestone?

[jonkoops]

I have to say that after fiddling around with a lot of these different options that the render prop is my favorite. It is easy to manipulate props coming in from the function and very expressive in defining the component since you can just add whatever props you want.

[dlabrecq]

My vote would be to use the component prop, which would allow components to be cloned, if necessary. It's an elegant way to allow users to override the underlying component.

For example, we use something like this in react-charts, a Victory pattern. We're cloning the typed containerComponent prop, which allows devs to override our default properties.

const ChartArea: React.FC<ChartAreaProps> = ({ containerComponent = <ChartContainer /> }: ChartAreaProps) => {
  const padding = { left: 10 };
  const container = React.cloneElement(containerComponent, {
    ...padding,
    ...containerComponent.props
  });
  ...
}

[gitdallas]

We had some discussion and it seems like there may be some places where component makes sense, other times where containerComponent or render might make sense.

We decided to have a spike in order to determine what changes should actually be made and where. @tlabaj

[jonkoops]

@nicolethoen @tlabaj looks like this one was prioritized for 'post V5' but never picked up, is this something to land in V6?

[tlabaj]

Hi Jon, our priority has been Penta theming for V6. We have tried to minimize other breaking changes to try and make it as easy as possible for consumers to pick up V6.