[Compiler]: Effects with always-new dependencies fire less with memoization applied

[guillaumebrunerie]

What kind of issue is this?

• React Compiler core (the JS output is incorrect, or your app works incorrectly after optimization)
• babel-plugin-react-compiler (build issue installing or using the Babel plugin)
• eslint-plugin-react-compiler (build issue installing or using the eslint plugin)
• react-compiler-healthcheck (build issue installing or using the healthcheck script)

Link to repro

https://github.com/guillaumebrunerie/react-compiler-bug

Repro steps

• Start the linked repro with npm run dev.
• Hover the text, and note that the hover text appears to the right.
• Uncomment the "use no memo" directive in src/App.tsx and notice that the hover text now appears below.

There are no lint errors, but the compiler causes the component to behave differently, so I guess it’s a bug in the compiler?

I noticed this issue when setting up the React compiler in one of my projects, and noticed that some components started behaving incorrectly. It is custom code that implements anchor-like positioning in JS, so somewhat messy code with refs, layout effects, and calculating the size and position of DOM elements. Suddenly one on the popups ended up positioned at (0, 0) with the compiler active, and adding "use no memo" in the affected component made the issue go away.

The repro here is a simplified version, which still behaves differently with and without the compiler. I am not aware of breaking any rules of React, but it is very possible I’m hitting some undefined behavior in the order in which effects run or similar.

I couldn’t simplify it much further, in particular

• removing useHover to always return true makes the issue go away,
• replacing the refContainer by the ref itself makes the issue go away,
• removing hoverRef makes the issue go away.

How often does this bug happen?

Every time

What version of React are you using?

19.1.0

What version of React Compiler are you using?

19.1.0-rc.1

[guillaumebrunerie]

I think I understand why it behaves differently. What I have is basically:

const App = () => {
    const [isHovered, setIsHovered] = useState(false);
    const ref = useRef();
    useEffect(() => {
        /* [...] */
    }, [{ ref }]);
}

In the unoptimized version, every time the hover state changes it triggers a rerender, which creates a new { ref } object, which triggers the effect.
In the optimized version, the { ref } object is memoized (as ref never changes), so the effect is never retriggered (and the position of the hover text is not updated).

So I guess the issue is that I’m using an effect incorrectly, but I still wonder if it’s something the compiler should be able to detect and skip the compilation of that component?

[suhaotian]

Put isHovered as the dependency here, because re-render is triggered by setState, but you use a trick (creating a new object every time) to make useEffect work.

[guillaumebrunerie]

Put isHovered as the dependency here, because re-render is triggered by setState, but you use a trick (creating a new object every time) to make useEffect work.

Yes, I know that would work, but that is not the point of this issue. The issue is that enabling the compiler silently breaks my app rather than skipping over the problematic component.

It's also a very fragile solution, because I would need to add every other state or things that could trigger a rerender in the dependency array. A better fix would be to implement the effect in a way that it doesn't depend on rerenders.

[josephsavona]

Thanks for posting! This is an interesting example to consider.

So I guess the issue is that I’m using an effect incorrectly, but I still wonder if it’s something the compiler should be able to detect and skip the compilation of that component?

This looks like the issue. Dependencies are meant to be values that are also referenced in the effect body, and a newly created value as a dependency will cause the effect to always re-run. This is something we should be able to detect in the compiler but haven't implemented yet.

[josephsavona]

Concretely, the feature request here is to detect that dependencies are invalid — always recreated — and skip memoization.

[szhsin]

Yes, I know that would work, but that is not the point of this issue. The issue is that enabling the compiler silently breaks my app rather than skipping over the problematic component.

It's also a very fragile solution, because I would need to add every other state or things that could trigger a rerender in the dependency array. A better fix would be to implement the effect in a way that it doesn't depend on rerenders.

Sounds like you want an ESLint rule to warn about incorrect usage, such as placing [{ ref }] in the dependency array.

Also, keep in mind that the dependency array is an optional argument in useEffect. If you don't provide it, the effect will run on every render. This will ensure the compiler not to break your code in this case.

[guillaumebrunerie]

In my actual use case I didn't put { ref } directly in a dependency array, but it was abstracted away in a custom hook:

const useMyCustomHook = (someValue) => {
  useEffect(() => {
    // ... uses someValue
 }, [someValue]);
};

// In a different file
const MyComponent = () => {
  const ref = useRef();
  useMyCustomHook({ ref, someOtherData });
};

I'm not even quite sure which part of this code is problematic. Is it the custom hook that should not pass an object to a dependency array? What makes matters worse is that the type of someValue is a discriminated union, so I can't really just inline all the properties of it in the dependency array without TypeScript complaining.