TIL: Don't expose async callback props in React

react-async-callbacks-considered-harmful.md

It's fairly common practice to expose callback props in React components. There are several reasons you might use callback props:

1. Notifying ancestors of some condition
2. Providing a way to render a surface inside your component, the so called render props.
3. A special case of render props is where the ancestor provides data that then feeds into what gets rendered.

My example builds on the last use-case. Consider the following hello world component:

interface HelloWorldProps {
  userId: string;
  onFetchName: (userId: string) => string;
}

function HelloWorld(props: HelloWorldProps) {
  const { userId, onFetchName } = props;
  const name = onFetchName(userId);
  return <h1>Hello {name}</h1>;
}

Why the convoluted way to fetch user name? Why not simply pass in name along with the userId?

A realistic case is where the userId and names for each user are sourced differently. This is a simplified example of some the API exposed in the Azure Communication Services UI library. For example the MessageThread component accepts a onRenderAvatar callback prop that maps the userId of the users sending messages to display name etc provided by the user of the library for each user.

Building upon that use-case, it is conceivable that the mapping from userId to names is stored by the user of the library in some backend and needs to be fetched asynchronously. To support this use-case naturally you might accept an asynchronous callback, like so:

interface HelloWorldPropsAsync {
  userId: string;
  onFetchName: (userId: string) => Promise<string>;
}

function HelloWorldAsync(props: HelloWorldPropsAsync) {
  // ...
}

Here be dragons!

This is, once again, a simplified example of a real API in the Azure Communication Services UI library. The UI composites in the library expose a callback prop to provide custom user data related to each userId in the call. This callback prop is asynchronous for the reason described above.

After working on several releases of the library, I have realized that exposing this asynchronous callback prop was a mistake.

Memoizing and performance

The typescript interface definition above hides a key aspect of the API contract between user of HelloWorldAsync and the implementation: How frequently is HelloWorldAsync.onFetchName called?

A simple answer would be: once per userId. HelloWorldAsync would fetch the name for a userId the first time it is needed and then cache it for future use. If this behavior satisfies your needs, none of the problems I discuss below apply to you. But I'd strongly encourage you to call out this expectation in the documentation for your component.

But what if the name for a userId can change during the lifetime of the application? This is a very realistic situation: Users can often change their display name, avatar etc in modern web applications. In that case, HelloWorldAsync has no way to know when the name for a userId changes. HelloWorldAsync must call onFetchName each time it is rendered to fetch the latest name for relevant userIds. This still does not guarantee that a change in name is reflected in the UI immediately -- HelloWorldAsync must render for the name to be updated. But the end result is acceptable in many cases.

In this case, the onus is on the user of HelloWorldAsync to memoize their implementation of onFetchName to avoid unnecessary rerenders of HelloWorldAsync.

Turns out, calling an asynchronous callback each time HelloWorldAsync renders is... hard:

function HelloWorldAsync(props: HelloWorldAsyncProps) {
  const { userId, onFetchName } = props;
  
  // Default name.
  const [name, setName] = useState('');
  // No dependency array so that this useEffect triggers *after every render*.
  useEffect(() => {
    (async () => {
      const newName = await onFetchName(userId);
      if (shouldUpdate(name, newName)) {
        setName(name);
      }
    })();
  });
  
  return <h1>Hello {name}</h1>;
}

Let's unpack what happened:

• Because onFetchName is asynchronous, it can not be called when HelloWorldAsync is rendered. Instead, it must be called via an immediately invoked asynchronous function expression (IIFE) in a useEffect block. The only way for the asynchronous expression to update the UI is by forcing another render -- enter useState. HelloWorldAsync must track the returned name via a state variable to trigger a UI update when the name is fetched.
  • Aside: Technically, the IIFE could be triggered inline in HelloWorldAsync because it isn't awaited. But it is better practice to trigger asynchronous updates via a useEffect block.
• This introduced a very risky potential render loop: Each time HelloWorldAsync renders, it triggers the IIFE, which potentially updates name which triggers a render... an accident waiting to happen. The user of HelloWorldAsync could easily trigger this render loop if onFetchName returned an object instead of a string -- by returning a new object each time with the same content.
  • Thus, the failure to memoize the return value of onFetchName is not "merely" worse performance, the application freezes as HelloWorldAsync enters a render loop.
  • The implementation above provides basic guardrails in the form of shouldUpdate check. It deep compares the new return value from onFetchName with the old value, and only updates the state if the values differ.

If you find the implementation of HelloWorldAsync ugly, I agree with you. An API that make it easy to step into a render loop, and having to break render loops via deep equality comparison of objects does not fit the React paradigm.

Stay synced

My recommended fix? For those of you wiser than me (or who don't have API backward compatibility requirements): Don't use an asynchronous callback.

Everything, and more, that the API above achieves can be achived with the simpler synchronous API I started with. It is always possible to convert a synchronous callback into an asynchronous one, but asynchronous callback flows upstream unchallenged.

Note: References required ;)

It is possible to use HelloWorld with an asynchronous data backend storing names as long as HelloWorld calls onFetchName on each render:

• Trigger any necessary data fetch in onFetchName, and cache the result as necessary.
• Return the cached result, and default if no result is yet available.

This essentially moves the async-fetch-and-cache outside HelloWorldAsync to the consumer of the component. So... am I happy because I need to do less work as the library author and the consumer gets to work hard? No. There are distinct advantages to the synchronous API:

• The harder to understand asynchronous API isn't forced on consumers that don't need it. Looking at the API of HelloWorldAsync, would you have guessed the non-trivial rendering behavior?
• This API allows the consumer to pick the default value to use when the asynchronous data fetch hasn't completed. There was no way to do that in the asynchronous API at all.
• The render loop is no longer possible. onFetchName is called while HelloWorld is already being rendered and the returned value is immediately used without ever triggering another render. The effect of a failure to cache the return value of onFetchName depends on how it is used inside HelloWorld -- it may cause more sub-components to render than necessary in the current render loop, but it will never trigger another render. Not memoizing the value causes worse performance. That's well within the React paradigm.