react icon indicating copy to clipboard operation
react copied to clipboard

[Compiler Bug]: Mutating a ref returned from a function warns

Open atomiks opened this issue 1 year ago • 3 comments

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)
  • [X] 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://playground.react.dev/#N4Igzg9grgTgxgUxALhAMygOzgFwJYSYAEUYCAsgJ4BKCaAFAJRHAA6xRMCOsxpCtBowDc7AL7t2GbPkJEAEggA2SiExbsiROITA5OdIgF4SZKoKaiO-AKJo0CXPXVGAfBo5auaAHRxYXJj6JgDkOAh6IVZaYiLiIGJAA

Repro steps

When consuming mutable refs from custom hooks (or via props), the consuming components should be allowed to mutate them inside effects/event handlers.

function useMyRef() {
  return useRef('test');
}

function Hello() {
  const ref = useMyRef();
  useEffect(() => {
    ref.current = 'next';
//  ~~~ InvalidReact: Mutating a value returned from a function whose return value should not be mutated. Found mutation of `ref` (8:8)
  });
}

How often does this bug happen?

Every time

What version of React are you using?

19

atomiks avatar May 21 '24 12:05 atomiks

Thanks for posting! The cause is very similar to https://github.com/facebook/react/issues/29160#issuecomment-2118621984 — the compiler currently assumes that only values that come directly from useRef() are refs. As with #29160 , we will likely need to explore a heuristic for understanding which values may be refs, based on the ecosystem convention of "ref" or "-Ref".

josephsavona avatar May 30 '24 00:05 josephsavona

we will likely need to explore a heuristic for understanding which values may be refs, based on the ecosystem convention of "ref" or "-Ref".

I'm looking into adding this. Assigning this to myself

gsathya avatar Jun 03 '24 20:06 gsathya

The error you're encountering happens because mutating a prop or a hook argument directly inside a hook is not allowed in React. The error arises because the target ref passed to the useResizeHandle hook is being directly mutated, which React warns against.

To resolve this, you should avoid directly modifying the target ref and instead use a local variable to track changes or update the target element in a way that doesn't involve directly mutating the passed reference.

Modified one: ` import * as React from 'react';

export function useResizeHandle( target: React.MutableRefObject<HTMLDivElement | null>, ) { const bar = React.useRef<HTMLDivElement>(null);

React.useEffect(() => { function resizeObject(event: MouseEvent | TouchEvent) { if (bar.current && target.current) { bar.current.style.width = "10px"; target.current.style.width = "10px"; // Make sure you are not mutating target itself but the element it refers to } }

document.addEventListener('mousemove', resizeObject, { passive: false });
return () => {
  document.removeEventListener('mousemove', resizeObject);
};

}, [target]); // target ref is a dependency here, but we don't mutate it directly

return { dragging: true, }; }`

Key Changes on the above code: Avoid Direct Mutation instead of directly mutating target or target.current, the code now modifies the style properties of the element referred to by target.current, which is allowed.

Check for Existence and Added a check to ensure that bar.current and target.current are not null before accessing their properties.

Note: 1.Make sure that you are not using React version 19, as there is no such version at the time of writing. The latest stable version is React 18. You might want to confirm which version you are using. 2.Ensure that ESLint is configured correctly with appropriate rules to avoid these kinds of errors in the future.

maddymathan avatar Aug 12 '24 15:08 maddymathan

Here's another variant of this issue - passing ref as a prop to a child component causes Compiler to not recognize it as a ref.

https://playground.react.dev/#N4Igzg9grgTgxgUxALhAMygOzgFwJYSYAEACgIYwKY4AUAlEcADrFFyFg5EAOZYYCACYAlBGiIBeIlAGi0NNGQA2AuixZEilHLGIAeAMIALPEsE8+AkWInBe-IXIC+RAPQA+Fk-WYM2fIRExqaCNMAWDtbiTgzMrOyYnESEADKEAOYklPySRPSS7owamhFWcgB0cLCU1Lk4MFAIANzF3pjFMghpmJnZYDSpGVkI-HQtmE4gTkA

If you set useRef directly in <Child /> it works as expected:

https://playground.react.dev/#N4Igzg9grgTgxgUxALhAMygOzgFwJYSYAEACgIYwKY4AUAlEcADrFFyFg5EAOZYYCACYAlBGiIBeIlAGi0NNGQA2AuixZEilHLGIAeAMIALPEsE8+AkWInBe-IXIC+RAPQA+Fk-WYM2fIRExqaCNMAWDtZoyEQA+kRODMys7JicEVZyktKyYgrKqj6aqemEADKEAOYklPzZ9JLujBqaGY5iAHRwsJTU2TgwUAgA3C3emC0yCBWY1bVgNOVVNQj8dKOYTiBOQA

aeharding avatar Oct 30 '24 21:10 aeharding

This should be closed by #29916 - thanks @gsathya for fixing!

poteto avatar Nov 13 '24 22:11 poteto