facebook / react

The library for web and native user interfaces.
https://react.dev
MIT License
229.52k stars 47k forks source link

Bug: eslint-plugin-react-hooks treats a non-react `use` function the same way as `React.use` #31237

Open artaommahe opened 1 month ago

artaommahe commented 1 month ago

React version: 18.3.1 eslint-plugin-react-hooks: 5.0.0

Steps To Reproduce

  1. install eslint-plugin-react-hooks 5.0.0
  2. write this code
    function smth(use: () => void) {
    use();
    }
  3. get an error React Hook "use" is called in function "smth" that is neither a React function component nor a custom React Hook function. React component names must start with an uppercase letter. React Hook names must start with the word "use".eslint[react-hooks/rules-of-hooks](https://reactjs.org/docs/hooks-rules.html)

We got this error with use function in the callback of playwright's test.extend()

const test = base.extend<{ track: E2EGeneratedTrackFragment }>({
  track: async ({}, use) => {
    // --------------^ local `use` function
    const track = await generateTestTrack('e2e');
    await addTrackToArcade(track.id);
    await use(track);
    // -----^ the error is here
    await deleteTestTrack(track.id);
  },
});

Link to code example: -

The current behavior

any custom use function is treated like React.use

The expected behavior

only use() function from react is linted by those rules

Additional

There is no error with eslint-plugin-react-hooks v4.6.2

eps1lon commented 1 month ago

Is this documented by Playwright to be named use?

artaommahe commented 1 month ago

yes

export type TestFixture<R, Args extends KeyValue> = (args: Args, use: (r: R) => Promise<void>, testInfo: TestInfo) => any;

also every example on this page https://playwright.dev/docs/test-fixtures#with-fixtures

And it's not just about playwright usage, it's more about preventing developers from using use as a custom function name anywhere in the react codebase with this rule enabled

leggomuhgreggo commented 6 days ago

My grandpa is here blustering at me saying "it's kind of amazing that the decision to name a hook use made it through review"

He sayin "Nobody thought about how the ESLint enforcement would effectively hijack an entire verb?" "No rule config to enable exceptions?"

I told him "Peepaw, just write your own ESLint rule? You don't have to use use"

(At that point, we looked at each other in unspoken agreement that having to now say 'use use' is a deeply unfortunate state of affairs)

And then he got quite loudly (when he gets loudly it gets very loud indeed) Lot of old timey swears and something about how he "doesn't have the time to fiddle with ESLint rules and nested glob patterns for the rest of his life"

He's a bit of a firecracker. I'm trying to reason with him because of course I can see both sides. But I thought I'd share since it's interesting feedback.

leggomuhgreggo commented 3 days ago

What if the rule logic were enhanced such that — when used as a an object method — it will only apply when the object is React?

Something like:

Applies

React.use()

Does not apply

AnyOtherObj.use()
Rel1cx commented 4 hours ago

What if the rule logic were enhanced such that — when used as a an object method — it will only apply when the object is React?

Something like:

Applies

React.use()

Does not apply

AnyOtherObj.use()

In fact, it is quite possible to determine whether that use is from React at static analysis level, and not only when used as a an object method. I've created a debug rule to prove it works.

Xnip2024-11-22_21-37-46