Updating Monaco broke Fastly Fiddle: here's how I solved it with useCallback in React
Tags: react, javascript, frontend
Published: 2023-04-19 15:07:23

My colleague [Dora](https://doramilitaru.com/) and I recently updated [Fastly Fiddle](https://fiddle.fastly.dev/)'s dependencies, and we suddenly found that user input in our code editor was erratic and unusably slow.  We fixed it by moving some state from the component into a local variable, persisted across renders thanks to [useCallback](https://react.dev/reference/react/useCallback).

This is what we saw in the UI:

![Keystrokes disappear on entry](https://dev-to-uploads.s3.amazonaws.com/uploads/articles/anf4rjc9le5aoejwqe36.gif)

Popping open the network panel, we can see a request to the server for every keystroke, 1 second after the key was pressed:

![Lots of network requests](https://dev-to-uploads.s3.amazonaws.com/uploads/articles/yiibwznmavl2oryaiwmk.gif)

Ouch. Our debounce is clearly broken - and not only that but we're also supposed to be cancelling save requests that are already in progress if a new keystroke is made, and that clearly isn't working either.

It turns out that the bottom line is, **if you read state in a function that's been [memoized](https://codeburst.io/understanding-memoization-in-3-minutes-2e58daf33a19), you will get an out of date version of that state.**

In our case, [Monaco](https://microsoft.github.io/monaco-editor/), the code editor component, was memoizing a function, invoking an outdated version of it, and as a result we were never able to tell that a save operation was already in progress, so we didn't cancel the existing operation, and cued up a new one.

## The problematic code

Many performance problems and behavioural bugs in React applications seem to stem from a too-hazy understanding of what causes a component to re-render, what happens when a component re-renders, and in what context a function is being invoked. Fortunately I am friends with [Ivan Akulov](https://twitter.com/iamakulov) who is basically web performance in human form, and he helped me figure this out.

In Fastly Fiddle, we have a state variable `saveOperation` which is read and updated by a `saveHandler` function, and which is also passed down into child components:

```js
const [saveOperation, setSaveOperation] = useState();

const saveHandler = async (userInput) => {
  if (saveOperation) saveOperation.abort();
  const debounceDelay = abortableWait(DEBOUNCE_DELAY);
  setSaveOperation(debounceDelay);
  await debounceDelay;
  ...
  setSaveOperation(null);
}

render(<Main onSave={saveHandler} />)
```

We're trying to wait for an idle period before saving the user's work (a 'debounce').  Every time we see new keystrokes, we abort the pending save - or any in-flight API request that is already in progress.

We're doing this using an [abortable promise pattern](https://zzdjk6.medium.com/a-simple-implementation-of-abortable-promise-using-abortcontroller-with-typescript-2c163ee050e8) - there's probably a more idiomatic way to do this in React but this seemed to work pretty well.  `saveHandler` is going to get redefined every time the component renders, but that should be fine - `saveOperation` is component state, which will persist across renders.  Right?

## Memoization problem

However, there's a risk - if anything further down the component tree memoizes the reference to `saveHandler`, we might end up calling an out of date version of the function which has captured an out of date version of `saveOperation`.

Turns out, this is exactly what's happening.  After our dependency update, we discovered that when keystrokes were entered into a code editor component powered by [Monaco](https://microsoft.github.io/monaco-editor/), the resulting invocation of `saveHandler` found `saveOperation` to be `undefined` every time. 

![saveOperation is always undefined](https://dev-to-uploads.s3.amazonaws.com/uploads/articles/fgs94k345cgkhx26gol8.png)

The `<Main>` component eventually passes the `saveHandler` function to Monaco like this:

```js
<MonacoEditor
  theme="fiddle"
  key={props.codeContext}
  options={monacoOptions}
  value={props.value}
  onChange={props.saveHandler}
/>
```

The new version of Monaco then memoizes the `onChange` callback, which means that when keystrokes are entered into the editor, the `saveHandler` that gets fired is one that doesn't have the latest value of `saveOperation`, so we don't know there's a save already in progress, so we don't cancel it, and all hell breaks loose.

## Progress state is an antipattern?

So what's the solution?  I could investigate why Monaco is memoizing the handler, and maybe try and convince it to not do that, but this problem highlights that using `useState` to store `saveOperation` is maybe not the best idea in the first place.  Apart from making the function dependent on data in the parent context, using state also means we *re-render the component every time we update it*.  Since `saveOperation` doesn't affect the component's render output, that doesn't seem right.

I've decided to call this kind of data "progress state", since it effectively carries data forward from one invocation of a function to the next, to allow a subsequent invocation to adjust its behaviour based on knowing where we left off in the previous one.

`useState` is the most obvious way to do this, but it's not a good one.  The React-y way to do this is actually `useRef`:

## Solution 1: useRef

```js
const saveOperation = useRef(null);

const saveHandler = async (userInput) => {
  if (saveOperation.current) saveOperation.current.abort();
  const debounceDelay = abortableWait(DEBOUNCE_DELAY);
  saveOperation.current = debounceDelay;
  await debounceDelay;
  ...
  saveOperation.current = null;
}
```

Updating `saveOperation` now won't trigger a render, and this will work even if `saveHandler` gets memoized.

The reason this works is the same reason `useRef` variables always have that weird `.current` thingy hanging off them.  The variable created by `useRef` is immutable, so capturing it into a function is fine, because we know it cannot be reassigned: the value of `saveOperation` (which is an object) *will never change*.  However, the *properties* of that object *can* change, and a function that has captured a reference to the object will see those updated properties.  That's why refs have a `.current` property.

## Solution 2: useCallback closures

React's `useCallback` hook creates a reference to a function and then reuses it in subsequent renders of the component, rather than redefining the function every time.  We figured, you could use this with an Immediately Invoked Function Expression (IIFE) to capture a value that persists between renders:

```js
const saveHandler = useCallback((() => {
  let saveOperation;
  return async (userInput) => {
    if (saveOperation) saveOperation.abort();
    saveOperation = abortableWait(DEBOUNCE_DELAY);
    await saveOperation;
    ...
    saveOperation = null;
  };
})(), []);
```

Now, when the component is first rendered, React will execute the IIFE, and assign the resulting function to `saveHandler`.  The returned function has access to `saveOperation` which is trapped in a closure created by the IIFE.

I like this solution because:

1. it leans more on fundamentals of JavaScript rather than relying on the framework to solve the problem
1. it creates a tighter scope for my data (`saveOperation` is not accessible outside of `saveHandler`)
1. It's the simplest use of `saveOperation` - no need to use a setter function, and no need to use a `.current` property.

However, [React's docs on `useCallback`](https://react.dev/reference/react/useCallback#caveats) have this caveat:

> In the future, React may add more features that take advantage of throwing away the cache — for example, if React adds built-in support for virtualized lists in the future, it would make sense to throw away the cache for items that scroll out of the virtualized table viewport. This should match your expectations if you rely on useCallback as a performance optimization.

So in future, my callback might get redefined more often... but for a use case like debouncing, it seems like that would still be fine most of the time.

## Conclusion

There are some nice lessons in this debugging exercise: Don't overuse `useState`. Learn to love `useRef`. And it's nice that React has a solution to this *and* that we can also solve it using JS language primitives.

Fine, go ahead and flame me for doing React wrong. I don't care, React is weird anyway.