Reducing our reliance on legacy React APIs

[joshfarrant]

Current state

According to the React documentation, some of the React APIs that we leverage in Primer Brand are considered legacy. Below I've listed all of APIs which are currently considered legacy by React, alongside the number of times each API is used in the Primer Brand codebase, the number of components using the API, and the RegEx used to gather this data, in case you want to verify this yourself.

API	Total occurrences	Components using API	RegEx
Children	92	31	(React\.|\w)Children\.
cloneElement	56	27	cloneElement
Component	0	0	Component
createElement	2	2	createElement
createRef	0	0	createRef
isValidElement	115	34	isValidElement
PureComponent	0	0	PureComponent

At this point in time, these APIs aren't considered deprecated, meaning there are no plans to remove them from React itself, but they are considered legacy.

"Legacy APIs - Exported from the react package, but not recommended for use in newly written code."
— React documentation

I propose that we stop relying on these APIs when building new components, and instead leverage more modern alternatives. I believe that this will have the added benefit of making our code easier to reason about, less fragile, and more flexible.

Replacing createElement

This function is only used in two places in our codebase and can be easily removed. Both of these occurrences can be replaced by replacing the call to createElement with the equivalent JSX.

Replacing Children, cloneElement, isValidElement

These three APIs tend to be used hand-in-hand — every file which uses isValidElement also uses Children and/or cloneElement — so it makes sense to consider them holistically.

These APIs are commonly used to solve a set of related problems around component composition and child manipulation. In Primer Brand, they're primarily used to:

• Ensure children meet certain criteria before rendering
• Transform children by adding or modifying props
• Enforce specific component structures or hierarchies
• Handle compound components

However, these approaches come with several drawbacks:

• Changes to child structure can break components unexpectedly
  • Wrapping elements in additional containers can interfere with child detection
  • Order-dependent children create rigid APIs that are prone to errors
• Iterating over and transforming children adds runtime overhead
  • Each cloneElement call creates a new element
  • Complex child manipulation can lead to unnecessary re-renders
• Documentation needs to be very explicit about supported child structures

Proposed solution

Instead of relying on these legacy APIs, I propose an approach which utilises render props, which are the React documentation's recommended alternative to these legacy APIs.

We already support render props in a few places within the codebase, so the core pattern is one that's already established within Primer Brand. At the moment though, we don't use render props to their full potential and so still end up supporting these legacy APIs alongside them.

By embracing in the functional nature of render props we can replace the vast majority of our usage of these deprecated APIs.

Example

Lets take our current Card component as an example.

Along with the core Card component, we also export Card.Image, Card.Label, Card.Icon, Card.Heading, and Card.Description. A typical card might look something like this.

<Card href="https://github.com">
  <Card.Icon icon={<CopilotIcon />} color="green" hasBackground />
  <Card.Label color="blue-purple">Beta</Card.Label>
  <Card.Heading>Code search & code view</Card.Heading>
  <Card.Description>
    Enables you to rapidly search, navigate, and understand code, right from
    GitHub.com.
  </Card.Description>
</Card>

The Card component uses the legacy Children, cloneElement, and isValidElement APIs to iterate through each child and filter out invalid children (view code). It then clones the Card.Heading and applies a few additional props (view code).

This same behaviour can be achieved using render props.

<Card
  href="https://github.com"
  renderIcon={() => <Card.Icon icon={<CopilotIcon />} color="green" hasBackground />}
  renderLabel={() => <Card.Label color="blue-purple">Beta</Card.Label>}
  renderHeading={props => <Card.Heading {...props}>Code search & code view</Card.Heading>}
  renderDescription={() => (
    <Card.Description>
      Enables you to rapidly search, navigate, and understand code, right from GitHub.com.
    </Card.Description>
  )}
/>

You can see what the implementation of this modified Card component looks like in the joshfarrant/render-props branch, along with an example of the working component in Storybook.

In the example above, the Card component no longer accepts children, and instead utilises render props to create slots for child components to fit into. Each of those slots is passed a function returns the component to be rendered into that slot.

The benefit of using functions (renderIcon={() => <Card.Icon />), rather than just passing the component straight in as a prop (renderIcon={<Card.Icon />) is that the Card component can provide additional props to be spread onto the child component, as we do above with the renderHeading function. The props provided by renderHeading contains the same props which were previously added to the Card.Heading using the deprecated APIs.

Another benefit of this approach is that it's easier to apply more rigorous typings to the component. For example, in our current Card implementation a consumer can render a Card without a Card.Heading. This not only goes against our interface guidelines, but it also breaks the core behaviour of the component and stops the Card from being clickable. There is no feedback to the consumer that this is an issue.

Using the render props approach, we can mark the renderHeading function as a required prop, meaning that consumers will get immediate feedback from TypeScript that they're missing a required prop.

This approach still allows us to do runtime checks to ensure that the components passed to the render props are of the type they should be. For example we can check that renderIcon().type === Card.Icon and take some action if that evaluates to false.

This approach will also allow us to have more control over how and where individual child components get rendered. We have some components where the order that the consumer provides the children in matters, such as the Checkbox component, and switching to using render props would remove that category of issue entirely.

Additionally, explicit render props will remove the need for manual checks to identify the type of child components, as in this example, since children will no longer be mixed together when they're provided to the component, and instead will already be separated out by the consumer.

Summary

My recommendation is that we start moving away from legacy React APIs when building new components and embrace render props going forward. Render props are the React-recommended approach and should provide the same functionality as our current approach.

I'd appreciate feedback and input on all of the above 🙂

[joshfarrant]

Just following up from a conversation I had with @rezrah outside of this discussion where we discussed whether context could be used to replace much of our use of Children, cloneElement, and isValidElement, since context is also listed as an alternative to these APIs alongside render props in the React documentation.

After reflecting on this, I think that there are two different scenarios for which we rely on these legacy APIs. To avoid burying the lede, here are the two scenarios along with, in my opinion, the patterns that can be used to achieve them.

Scenario	Possible using render props?	Possible using context?
Modifying the props of provided children	✅	✅
Rendering certain children into certain locations	✅	❌

Modifying the props of provided children

In this scenario, we don't really care what children are passed, we're just going to modify their props slightly and render them. We do this in a handful of places, but I'm going to take ButtonGroup as an example (view code).

ButtonGroup takes the children that are provided to it and modifies the props of each of them. It doesn't do anything unique to one child versus another, based on their type.

This can be achieved nicely using both context and render props.

// Context
<ButtonGroup>
  <Button>1</Button>
  <Button>2</Button>
  <Button>3</Button>
</ButtonGroup>

// Render props
<ButtonGroup
  renderButtons={props => (
    <Button {...props}>1</Button>
    <Button {...props}>2</Button>
    <Button {...props}>3</Button>
  )}
/>

In this scenario (where there is only ever a single slot into which the children are rendered) there's not really much difference in the approaches; whatever you can do with one, you can do with the other.

Scenario	Possible using render props?	Possible using context?
Modifying the props of provided children	✅	✅

Rendering certain children into certain locations

In this scenario, we need to modify the behaviour of the component itself based on the children that are passed, or fundamentally treat one child differently compared to another based on its type, for example rendering one child into one location and another child into another. Some good examples of this are Statistic, Accordion, and River (and its sub-component, RiverContent). I'll focus on Accordion for this example.

Accordion accepts an Accordion.Heading and an Accordion.Content child. Because both of these are passed as children, we have to find both the Accordion.Heading and Accordion.Content within the children array (using the deprecated APIs). In some components — for example Statistic — we do something slightly different depending on whether we do/don't have a particular child (view code).

Context doesn't really help here. Context is useful for providing props from a parent component to children (direct children, or deeply-nested children). It doesn't concern itself with rendering, or deciding where specific components can be rendered. It can't be used to solve this problem.

Render props, on the other hand, function like slots. You can use a render prop to decide where, how, and with what props something gets rendered.

// Render props
<Accordion
  renderHeading={props => <Accordion.Heading {...props}>Heading</Accordion.Heading>}
  renderContent={props => (
    <Accordion.Content {...props}>
      <p>Some description</p>
      <p>Some description</p>
    </Accordion.Content>
  )}
/>

// Context
// ???

Scenario	Possible using render props?	Possible using context?
Rendering certain children into certain locations	✅	❌

Summary

As soon as we start inspecting the type of components — which is something we do quite often — context is no longer useful to us in solving the underlying issue. We'll still have to inspect the children and branch based on the type of each child, which will require us to use the legacy APIs that I'm proposing we avoid.

On the other hand, render props are flexible enough to work well in either scenario; they can provide a single slot into which children can be rendered (eg in ButtonGroup), or they can provide multiple slots for rendering different components in different places. The implementation of both scenarios is identical, it's just a question of whether you have one, or multiple render props. Their only downside that I can see is that they may be less familiar to consumers, despite the fact that they are an officially supported pattern in the React docs, and something that we use elsewhere in the codebase already.

As it stands, I believe render props are the only option which will allow us to render specific children into specific locations, but I'd love to explore any options I may have missed.

If we're really tied to the familiarity that context can provide for components which support a single child slot, then maybe there is a situation where we implement a convention that states that we use context for components which only have a single child slot, and render props when there are multiple child slots. This could, however, be perceived as inconsistent, even if there is a clear rule which denotes which approach is used. Also, if a component were to move from having one child slot to multiple, then it would require a refactor (and a breaking change) to migrate it from context to render props.

As context doesn't allow us handle both scenarios (without relying on these legacy APIs) then it may be simpler and more consistent to adopt render props whole-hog, and give a primer (excuse the pun) on render props in our docs for those who haven't used them before.

As always, I welcome feedback, corrections, and questions on all of the above 💖

[joshfarrant]

I'd also like to briefly touch on the ergonomics of render props, as they may feel a bit less familiar to consumers.

Take my Accordion example above, and lets say you're rendering it as part of a larger component, MyComponent.

export const MyComponent = () => (
  <Accordion
    renderHeading={props => <Accordion.Heading {...props}>Heading</Accordion.Heading>}
    renderContent={props => (
      <Accordion.Content {...props}>
        <p>Some description</p>
        <p>Some description</p>
      </Accordion.Content>
    )}
  />
)

Passing functions which return JSX into the renderHeading and renderContent props may feel a bit odd at first if it's not a pattern you've come across before. It may not be immediately obvious that the functions that you're passing to renderHeading and renderContent are just components; they accept a props object and they return some JSX!

This means that you could treat them as standalone components alongside MyComponent if you preferred

const AccordionHeading = props => <Accordion.Heading {...props}>Heading</Accordion.Heading>
const AccordionContent = props => (
  <Accordion.Content {...props}>
    <p>Some description</p>
    <p>Some description</p>
  </Accordion.Content>
)

export const MyComponent = () => (
  <Accordion
    renderHeading={AccordionHeading}
    renderContent={AccordionContent}
  />
)

This example and the first example I shared are functionally identical. Note that the components AccordionHeading and AccordionContent aren't exported. They're not shared components which are meant for general use, they're just an implementation detail which may make the core component (MyComponent) more readable.

It's also worth noting that the names of the props — renderHeading and renderContent — are arbitrary too. If it made things clearer they could simply be called Heading and Content.

export const MyComponent = () => (
  <Accordion
    Heading={AccordionHeading}
    Content={AccordionContent}
  />
)

This is all a bit of a tangent from the original purpose of this discussion, however I think it becomes relevant when comparing APIs for render props to those of the more familiar Contexts.