Bug: Inefficiency when hooks return multiple values

[mohebifar]

React compiler inefficiently re-evaluates code when dependencies that do not affect the result have changed. Specifically, when the hook returns multiple values.

Input code:

export default function Test() {
  const [data, refetch] = useData();

  const y = deepClone(data);

  return (
    <div>
      <button onClick={refetch}>
        Refetch
      </button>
      <span>
        {JSON.stringify(y)}
      </span>
    </div>
  );
}

Check the output JS here - Link to Playground

Steps To Reproduce

1. Use the provided original code in a React component.
2. Observe the generated code by the React compiler.
3. Note that deepClone(data) is re-evaluated when refetch changes.

Check the playground link above ^

The current behavior

• The line const y = deepClone(data); is re-evaluated whenever refetch changes, even if data remains the same.
• This inefficiency arises because the compiler does not memoize y independently from other variables like t1.

The expected behavior

y should be memoized separately to avoid re-computation when only refetch changes and data remains the same.

[josephsavona]

Thanks for posting. A simplified version of what's going on here is this:

const [data, refetch] = useData();
const y = deepClone(data); // y becomes mutable here
const t0 = <button onClick={refetch}> // this button gets memoized with `y`, so `refetch` becomes a dependency
const t1 = maybeMutate(y);  // y is last mutated here
return <>{t0}{t1}</>;

Ie, the compiler currently sees JSON.stringify(y) and thinks that it could be a mutation of y, and this affects the memoization. We have type declarations that understand the mutability semantics of various built-ins, but haven't added JSON.stringify yet (it's a fairly complex one to type because of the replacer callback). This is definitely on our radar to improve, but this is working as designed. Not memoizing in this way could cause us to miss mutations and produce incorrect results.

I'm going to close this since it's working as designed, but we will be working over time to type more builtins, including JSON.stringify.

[mohebifar]

Ok, probably the answer I was looking for is in:

this button gets memoized with y, so refetch becomes a dependency

So that means that React compiler does not auto memoize everything with the correct dependencies.

When you memoize y manually, you'd do:

const y = useMemo(() => deepClone(data), [data]);

while what React compiler does is more equivalent to:

const y = useMemo(() => deepClone(refetch), [data, refetch]); // With refetch also invalidating the memo'd value

This perhaps should be noted in the docs somewhere that you may need to memo by hand in such cases that "precise dependencies" matter.

[josephsavona]

So that means that React compiler does not auto memoize everything with the correct dependencies.

No. The compiler does a conservative analysis and uses the correct set of dependencies given the values that are being memoized. In this case, there are two values being memoized together, so we emit the union of their dependencies. That is the only correct thing to do.

There are alternative ways we could memoize — for example, entering, exiting, and re-entering memoization blocks — that have different tradeoffs. We may explore those later, but so far we've found that they have unacceptable tradeoffs such as high code size.