[eslint-plugin-react-hooks] allow configuring custom hooks as "static"

grncdr
创建于
2019-09-24 11:45:13

Do you want to request a feature or report a bug?

Feature/enhancement

What is the current behavior?

Currently the eslint plugin is unable to understand when the return value of a custom hook is static.

Example:

import React from 'react'

function useToggle(init = false) {
  const [state, setState] = React.useState(init)
  const toggleState = React.useCallback(() => { setState(v => !v) }, [])
  return [state, toggleState]
}

function MyComponent({someProp}) {
  const [enabled, toggleEnabled] = useToggle()

  const handler = React.useCallback(() => {
    toggleEnabled()
    doSomethingWithTheProp(someProp)
  }, [someProp]) // exhaustive-deps warning for toggleEnabled

  return <button onClick={handler}>Do something</button>
}

What is the expected behavior?

I would like to configure eslint-plugin-react-hooks to tell it that toggleEnabled is static and doesn't need to be included in a dependency array. This isn't a huge deal but more of an ergonomic papercut that discourages writing/using custom hooks.

As for how/where to configure it, I would be happy to add something like this to my .eslintrc:

{
  "staticHooks": {
    "useToggle": [false, true],  // first return value is not stable, second is
    "useForm": true,             // entire return value is stable 
  }
}

Then the plugin could have an additional check after these 2 checks that tests for custom names.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

All versions of eslint-plugin-react-hooks have the same deficiency.

11条回答
grncdr
回复于
2019-09-29 22:09:58
#1

I went ahead and implemented this (see above commit) just to see how it would play out in my own codebase. If anybody else feels like trying it, I've published it to npm under @grncdr/eslint-plugin-react-hooks. You will need to update your eslintrc to reference the scoped plugin name and configure your static hook names:

-  "plugins": ["react-hooks"],
+  "plugins": ["@grncdr/react-hooks"],
   "rules": {
-    "react-hooks/rules-of-hooks": "error",
-    "react-hooks/exhaustive-deps": "warn",
+    "@grncdr/react-hooks/rules-of-hooks": "error",
+    "@grncdr/react-hooks/exhaustive-deps": [
+      "error",
+      {
+        "additionalHooks": "usePromise",
+        "staticHooks": {
+          "useForm": true,
+          "useEntityCache": true,
+          "useItem": [false, true],
+          "useQueryParam": [false, true]
+        }
+      }
+    ],

(note the hook names above are specific to my app, you probably want your own)

If anybody from the React team thinks the idea is worth pursuing I'll try to add some tests and make a proper PR.

回复于
2020-01-09 20:48:21
#2

This issue 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.

VanTanev
回复于
2020-01-16 11:17:29
#3

This seems like a really great addition, would love to see it in react-hooks

grncdr
回复于
2020-01-19 09:32:28
#4

@VanTanev have you tried my fork? I've been using it since my last comment and haven't had any issues, but positive experience from others would presumably be interesting to the maintainers.

ramiel
回复于
2020-01-23 11:42:27
#5

Any news on this. It's very annoying now because you cannot use reliably this lint rule when you use custom hook, so you have to disable the rule leading to potential dangerous situations

kravets-levko
回复于
2020-01-23 11:58:14
#6

IMHO it would be great if this plugin could detect some common "static" patterns in custom hook, for example if custom hook returns result of useRef()/useCallback(..., [])/useMemo(..., []) etc.

ramiel
回复于
2020-01-23 14:20:11
#7

Indeed. Still there may be ambiguous situations and so having the ability to set it up through options could still be needed

garetmckinley
回复于
2020-01-28 16:22:12
#8

Commenting to bump this thread and show my interest. Working on a large codebase with lots of custom hooks means that this would allow us to more reliably use the hooks linter. I understand that the reason they might not want to allow this, is because it could enable people to introduce dangerous bugs into their apps. So maybe it's a feature that should be added with a large disclaimer.

It's pretty likely that the maintainers simply don't want to deal with bug reports that are related to people setting their hooks to static when they actually aren't static. A lot of people will misunderstand what it means to have static hooks.

grncdr
回复于
2020-01-28 17:34:30
#9

IMHO it would be great if this plugin could detect some common "static" patterns in custom hook, for example if custom hook returns result of useRef()/useCallback(..., [])/useMemo(..., []) etc.

This is way beyond the scope of what ESLint can do (it would require whole-program taint-tracking) so definitely not going to happen here.

I understand that the reason they might not want to allow this, is because it could enable people to introduce dangerous bugs into their apps. So maybe it's a feature that should be added with a large disclaimer.

It's pretty likely that the maintainers simply don't want to deal with bug reports that are related to people setting their hooks to static when they actually aren't static. A lot of people will misunderstand what it means to have static hooks.

I would totally understand this point of view, but until somebody from the React team replies, I'll keep hoping (and using my fork 😉).

ksjogo
回复于
2020-02-25 15:26:48
#10

@grncdr can you please point me to the source of your folk?

当前位于第1页,总计11 条回复

推荐相似问题

暂无更多