Treat Namespace.use() as a React Hook

[jamiebuilds]

In watching the presentation about building the new facebook.com I saw a reference to a method called:

Feature.use()

That was used conditionally, which was confusing because it looked like a hook to me.

I was actually surprised to find out that React does not consider Namespace.use() to be a hook at all, and even Namespace.useHook() was not considered a hook by the ESLint plugin because it isn't React.useHook().

I want to propose two things:

1. Namespace.useHook() should be considered a hook by the ESLint plugin
2. Namespace.use() should be considered a hook by React/ESLint plugin

I've opened this PR with both changes. Although I can easily update it to only include Namespace.useHook() if that is preferred.

But I would like to argue for also including Namespace.use() because the alternative for people wanting to write that is Namespace.useNamespace() which is just silly to write.

[threepointone]

Hi @jamiebuilds I miss you ♥️

[gaearon]

At the time we introduced the plugin internally there were too many false positives from this pattern. So we disabled it. Not that FB code is special — but if we had false positives, it’s likely others will too.

I think internal call sites have been removed by now. Maybe we can try again.

One particular example I’ve seen is older React Rourer History API. Like useQueries. Those aren’t Hooks. And it was convenient that you could make the lint rule ignore them by turning the history import into a namespace.

Another concern is that we really want to discourage people putting Hooks inside classes. Like SomeClass.use() or even worse, someInstance.use(). This can get very gnarly. So the push for plain functions as a convention is partially to discourage those.

[jamiebuilds]

And it was convenient that you could make the lint rule ignore them by turning the history import into a namespace.

I think someone would sooner reach for a eslint-ignore comment anyways.

At this point, React has basically reserved functions called use* within React apps, I wouldn't expect someone to name something use* which is not a hook.

The only place I can see it being annoying is in the intersection of other ecosystems, most notably when server-side rendering React code inside of an express app, express uses app.use(...) for its own middleware.

Another concern is that we really want to discourage people putting Hooks inside classes. Like SomeClass.use() or even worse, someInstance.use(). This can get very gnarly. So the push for plain functions as a convention is partially to discourage those.

Yeah I'm thinking about this more in the context of factory functions that create objects with hooks on them, like:

let Container = createContainer()

function Child() {
  let container = Container.useContainer()
  // ...
}

function Parent() {
  return (
    <Container.Provider>
      <Child />
      <Child />
    </Container.Provider>
  )
}

Someone could fairly easily destructure this, but it seems silly that a lint rule would force them to.

---

@threepointone eat my ass

[gaearon]

Oh yeah app.use() is a notable one.

[jamiebuilds]

I think this should also consider how the hook naming is being taught. This is often the definition I hear:

I often hear the X part of useX being implied, but taking this definition literally, it means /^use/ which would match use()

But more importantly, you often hear "any function" which would include namespaces methods.

[vikingair]

I use hooks a lot and have implemented various arbitrary complex custom hooks. Hence I noticed that mocking hooks for my tests might be necessary. But I don't want to mock the whole module only in order to use the important eslint hook rules.

Let me introduce another suggestion for namespaced hooks:

FooHooks.useBar

Foo.hooks.useBar

Following the pattern that the namespace ends with "Hooks" or the accessed hook is stored in some object called "hooks" AND the called function starts with "use".

@gaearon How do you think about that?

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution.

[jamiebuilds]

bump

[stale]

This pull request has been automatically marked as stale. If this pull request is still relevant, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize reviewing it yet. Your contribution is very much appreciated.

[gaearon]

So... We recently made the lint rule stricter, so that it doesn't let you call Hooks from classes. However, we still kept Namespace.useFoo() not being considered a Hook. We could change this in principle, but it means it would also affect a larger surface area now.

I think I'm fairly sure we don't want to treat Namespace.use() as a Hook. That's short enough that it encourages putting Hooks on objects, which often leads to misunderstanding like sharing closure state or class fields between them.

We could maybe accept Namespace.useHook though. Would you update this PR to do just that? Then I could run it internally and see the rate of false positives.

[vikingair]

So there would be a limit of one hook per namespace?

[gaearon]

No, I meant that let's give PascalCase.use[A-Z].* a try. Let's consider them all Hooks.

[vikr01]

@gaearon Maybe we can just make this an option for the rule?

[gaearon]

Options undermine conventions. If we think this is a good idea we should probably just do it.

[gaearon]

We merged a more limited version of this in #18722.