Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Compiler Bug]: Mutating a ref passed as argument from a hook warns #29832

Open
1 of 4 tasks
oliviertassinari opened this issue Jun 10, 2024 · 2 comments
Open
1 of 4 tasks
Labels
Component: Optimizing Compiler Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug Type: Bug

Comments

@oliviertassinari
Copy link
Contributor

oliviertassinari commented Jun 10, 2024

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://playground.react.dev/#N4Igzg9grgTgxgUxALhASwLYAcIwC4AEAVAQIZgEBKCpchAZjBBgQOQw12sDcAOgHYCEADxz4C9KPzpoI-AlDAJqYNAC8EACVL8AJgBsEACgEECeUjADmCPMiqc8AOgCyUCwCND1egHkPAFYIdAA8mgAqLgAyACJoAG4AooYYCPyEAD4E-FD6+gB8ADQCAJQEwKYEcHJghB6WBAC8DrTOisoI9GGRsQnJCKnp+UY5eSV8gvItdE7tifT0wXhGRmWN+eWVZpLSeLLyHKoa-kF0RgjxaXYELtBKiZfpBFnh0HAAFg9XZRVTZmb1GBOOCwDjpJy1ACehicAHc0Lo8O8mgReCAAIwABiwwjRE3+-ws1lswNBVwheGhCDhCKRKLRWJxeK2BAAvgIWboICDBs5SLpdF90lE0LU0ggYEZWBg7gMIJdWIUCId1AgTkslcACFhyKpLvZ6KR9Eo2eMWRw8LB5KsmhtfgSzFyeeSODLLkK8CKxfwJVKZe03QhFcqEEc1YElma-mz8WylQBtIk2PAAXSjZgtVs20d0MFIVisaH4VnseBgUAQxSmrIm7P4IFZQA

Repro steps

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) {
      bar.current.style.width = "10px";
      target.current.style.width = "10px";
// ~~ InvalidReact: Mutating component props or hook arguments is not allowed. Consider using a local variable instead. Found mutation of `target`
    }

    document.addEventListener('mousemove', resizeObject, { passive: false });
    return () => {
      document.removeEventListener('mousemove', resizeObject);
    };
  }, [target]);
  return {
    dragging: true,
  };
}

This seems like the same as #29196 but in reverse.

From what @josephsavona said:

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

This could make sense. I could see myself doing:

 export function useResizeHandle(
-  target: React.MutableRefObject<HTMLDivElement | null>,
+  targetRef: React.MutableRefObject<HTMLDivElement | null>,
 ) {

How often does this bug happen?

Every time

What version of React are you using?

v19

@atomiks
Copy link

atomiks commented Jun 15, 2024

It feels like checking if .current is accessed on a value rather than checking for a Ref suffix would be more robust, preventing the need to change the names of variables and make it backwards-compatible (although I think the Ref naming convention is best to use anyway).

@oliviertassinari
Copy link
Contributor Author

@atomiks agree with your two points. It makes a lot of sense to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Optimizing Compiler Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug Type: Bug
Projects
None yet
Development

No branches or pull requests

2 participants