[Compiler Bug]: false positive when using hooks in a nested component

NMinhNguyen 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

https://playground.react.dev/#N4Igzg9grgTgxgUxALhAgHgBwjALgAgBMEAzAQygBsCSoA7OXASwjvwFkBPAQU0wAoAlPmAAdNvjiswBANoBbaHVwJCAGnxgEudkpWEAuvgC8+KFoDKuMiv7lKWwePyTpBAJJ06CGAGEI8tjeyiZmWr5klJQARmRwANZ29IwsbJ7efgFBCMpCIs4uYQgAoiQkCIz8ecYAfPkShZrauvT6-LgwUAiCANwFLgC+GrIGveL9+DDasGwAPIRMAG41TF4+swD0C8t9EkP4I2N0BVO4M-izAGIQEK6BrDm4xsDpPv73wbgD+Bs1uwPjOi0BjMVj4a4QfiYGAQTBgYRiCSnc6zaGwsAAOikH0ePxq4gGIAGQA

Repro steps

The ESLint plugin for React Compiler seems to not recognise inner function components as components, so it reports an error when you try to use hooks inside such nested components. eslint-plugin-react-hooks does recognise this pattern and doesn’t warn about it.

Hooks must be called at the top level in the body of a function component or custom hook, and may not be called within function expressions. See the Rules of Hooks (https://react.dev/warnings/invalid-hook-call-warning).

I understand that defining a component like this isn’t ideal because useMemo / useCallback don’t have strong guarantees or can get invalidated due to dependencies changing, leading to remounts so would be good to know if code should be rewritten away from this pattern or whether this is a false positive that will eventually get resolved.

How often does this bug happen?

Every time

What version of React are you using?

19.0.0-rc-3f1436cca1-20240516

josephsavona commented 1 month ago

Thanks for posting. The compiler does not yet support nested component definitions (components defined within another component). This is something we may add support for in the future depending on demand.

In the meantime though, we should provide a better diagnostic (“nested components not yet supported”) rather than throw a hook error.

RafaelJohn9 commented 1 month ago

@josephsavona

In the meantime though, we should provide a better diagnostic (“nested components not yet supported”) rather than throw a hook error.

Can I work on this, or it was just an afterthought

josephsavona commented 1 month ago

@RafaelJohn9 we're open to PRs to improve the warning!

RafaelJohn9 commented 1 month ago

@josephsavona, I will be working on it.

last-Programmer commented 1 month ago

we are also getting this kind of error in our application when using react-compiler. this is not the eslint warnings. happens actually while running the application

@NMinhNguyen Is this only eslint warning or error happens while running the application? thanks

josephsavona commented 1 month ago

@last-Programmer the error here is specifically reported by the ESLint plugin. It can be reported by the Babel plugin, but only if you’ve changed the plugin settings, which we don’t recommend.

So most likely you’re seeing a different error, can you create a separate issue please w info on how to reproduce? Thanks!

RafaelJohn9 commented 1 month ago

hey @josephsavona sorry, I did not notice earlier my PR has merge conflicts, I'll close it and make another PR. I do have a question, am I to add tests to the changes that I have made, and if I am to, what about the previous tests that were written? Thank you in advance.

RafaelJohn9 commented 3 weeks ago

@josephsavona

facebook / react