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]: Should support hooks named with symbols: use$, use_ #32473

Open
1 of 4 tasks
jmeistrich opened this issue Feb 25, 2025 · 3 comments
Open
1 of 4 tasks

[Compiler Bug]: Should support hooks named with symbols: use$, use_ #32473

jmeistrich opened this issue Feb 25, 2025 · 3 comments
Labels
Component: Optimizing Compiler Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug Type: Bug

Comments

@jmeistrich
Copy link

jmeistrich commented Feb 25, 2025

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/vladmiller/legend-state-react-compiler-bug

Repro steps

isHookName checks for valid hook names and requires alphanumeric names. I believe it should also support $ and _ since those are valid characters in JavaScript function names.

In Legend State we have a hook named use$ to consume observables, which I had thought would be compatible with Compiler. So this is currently breaking Legend State with the Compiler enabled. See issue: LegendApp/legend-state#477

I think the isHookName regexp should be changed to this to match valid function names in JavaScript.

/^use[A-Z0-9_$]*$/

Thanks 🙏

How often does this bug happen?

Every time

What version of React are you using?

19.0.0

What version of React Compiler are you using?

19.0.0-beta-e1e972c-20250221

@jmeistrich jmeistrich added Component: Optimizing Compiler Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug Type: Bug labels Feb 25, 2025
@Sardorbek-Kuvondikov
Copy link

+++

@mofeiZ
Copy link
Contributor

mofeiZ commented Feb 26, 2025

Hi @jmeistrich, thanks for reporting!
You're definitely correct in that our docs state Functions whose names start with use are called Hooks in React. However, our eslint rules considers only use[A-Z] named function calls as custom hooks. This was intended to promote readable and distinct hook naming and reduce false positives (e.g. userFilter(users)).

React Compiler needs to be very correct in its custom hook callsite analysis. This is because (1) under-detection leads to memoization of hooks and undefined behavior (as you're currently experiencing) and (2) false positives result in incorrect mutability analysis (i.e. anything flowing into a hook is considered "frozen" as hooks are pure). Both of these can lead to serious bugs, and we're currently erroring on the side of preferring under-detection (which is much simpler to catch).

Would LegendApp be open to renaming use$ to useLegend$ or something similar? This should also benefit all users on the official React rules-of-hooks eslint.

@facebook facebook deleted a comment from NeutronDisk Feb 26, 2025
@jmeistrich
Copy link
Author

Hi @mofeiZ, thanks for the detailed response!

I am trying to make Legend State 100% compatible and and correct with Compiler and all of the React rules, so I'll change it from use$ to something else if that's what's necessary. We already made a big change from having an observer HOC to requiring using a hook, in order to be Compiler compatible. But then I expected use$ would work because it's a function name starting with "use" as described in the docs. The name use$ works very will with our other naming conventions, but if it is specifically not supported then I'll rename it.

I understand the reasoning for trying to prevent false positives. But I'm curious what false positives are avoided by not allowing $? It seems like (outside of Legend State) there could be valid use cases like use$Value or use$Data which should not be considered false positives?

If the constraint is "Functions whose names start with use" that suggests that any valid JavaScript function name thats starts with "use" would work, including the two symbols valid in function names ($ and _). Although it's probably rare that users would name a function like that, it is very confusing that React's definition of a valid function name is different than JavaScript's definition. I wonder if the docs should specify the difference in definitions or the babel plugin should log a warning when a use* function is not considered an actual hook, because a non-hook hook being memoized completely breaks the components and it's unclear why.

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

3 participants