[Compiler Bug]: `useLayoutEffect` without dependency array should be allowed by either `react-hooks/exhaustive-deps` or `react-compiler/react-compiler`

Svish commented 1 month ago

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

N/A

Repro steps

Use useLayoutEffect without dependency array, for example:

export default function Breadcrumb({ children, to }) {
  const itemRef = useRef<HTMLLIElement>(null);
  const [isLast, setIsLast] = useState<boolean>();

  useLayoutEffect(() => {
    setIsLast(itemRef.current?.matches(':last-child') ?? false);
  });

  return (
    <li ref={itemRef}>
      <a to={to} aria-current={isLast ? 'page' : undefined}>
        {children}
      </a>
    </li>
  );
}

This will generate a react-hooks/exhaustive-deps warning for using useLayoutEffect without dependencies, but the dependency array is optional when you want the effect to run on every render, which I do in this case. Up till now I had just ignored this warning using // eslint-disable-next-line react-hooks/exhaustive-deps, but now this will then generate a react-compiler/react-compiler warning:

React Compiler has skipped optimizing this component because one or more React ESLint rules were disabled. React Compiler only works when your components follow all the rules of React, disabling them may result in unexpected or incorrect behavior

So... either you should fix the react-hooks/exhaustive-deps rule to allow effects without dependencies (useEffect seem to allow this, but not useLayoutEffect for some reason), or the compiler should be smarter about what disabled ESLint rules to take into account...

How often does this bug happen?

Every time

What version of React are you using?

react@18.3.1

What version of React Compiler are you using?

eslint-plugin-react-compiler@19.0.0-beta-8a03594-20241020

josephsavona commented 1 month ago

Thanks for posting. This is a known issue, we currently treat all exhaustive-deps suppressions the same, but we don't strictly have to bail out of compilation if the suppression is applied to an effect (whereas we do have to skip compilation if you have non-exhaustive deps on useMemo/useCallback).

cc @mofeiZ @mvitousek we talked about this recently

Svish commented 1 month ago

In my case it would be fixed by just allowing useLayoutEffect without a dependency array, like useEffect seem to do already. Kind of weird that exhaustive-deps treats those two differently, when they seem to have the exact same signature and behavior (except for when the effect is "scheduled"). 🤔

Joj501 commented 1 month ago

Fix

Joj501 commented 1 month ago

Fix ### 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

N/A

Repro steps

Use useLayoutEffect without dependency array, for example:
export default function Breadcrumb({ children, to }) {
  const itemRef = useRef<HTMLLIElement>(null);
  const [isLast, setIsLast] = useState<boolean>();

  useLayoutEffect(() => {
    setIsLast(itemRef.current?.matches(':last-child') ?? false);
  });

  return (
    <li ref={itemRef}>
      <a to={to} aria-current={isLast ? 'page' : undefined}>
        {children}
      </a>
    </li>
  );
}
This will generate a react-hooks/exhaustive-deps warning for using useLayoutEffect without dependencies, but the dependency array is optional when you want the effect to run on every render, which I do in this case. Up till now I had just ignored this warning using // eslint-disable-next-line react-hooks/exhaustive-deps, but now this will then generate a react-compiler/react-compiler warning:

React Compiler has skipped optimizing this component because one or more React ESLint rules were disabled. React Compiler only works when your components follow all the rules of React, disabling them may result in unexpected or incorrect behavior

So... either you should fix the react-hooks/exhaustive-deps rule to allow effects without dependencies (useEffect seem to allow this, but not useLayoutEffect for some reason), or the compiler should be smarter about what disabled ESLint rules to take into account...

How often does this bug happen?

Every time

What version of React are you using?

react@18.3.1

What version of React Compiler are you using?

eslint-plugin-react-compiler@19.0.0-beta-8a03594-20241020

facebook / react