[New] : Enforce useState hook destructuring and variable naming #2873

duncanbeevers · 2020-12-09T17:56:38Z

Ensure symmetric value and setter variable names are destructured from React.setState hook calls

Towards #2417

@karlhorky feel free to riff on this, or let me know how this compares to your own proof-of-concept.

Supports both useState and React.useState
Tested with TypeScript parser and useState<> type annotation
Robust against weird useState uses like destructuring too many items, or not destructuring at all
Fixable when 2nd destructured arg does not matching set<X> naming convention

@ljharb I included this in the Best Practices category, though perhaps a different categorization would be more appropriate.

Let me know if the documentation needs anything else, like more examples, or whether the laissez-faire approach to weird assignment is too forgiving.

karlhorky · 2020-12-09T18:54:58Z

Awesome @duncanbeevers ! I think you've probably put more effort into this than I have up until now - haven't found the time to learn what I would need to in order to tackle this 😬

I think it's looking good 🙌

@ljharb what are your thoughts?

ljharb · 2020-12-30T03:26:58Z

docs/rules/hook-use-state-names.md

+const [color, updateColor] = React.useState();
+```
+
+Examples of **correct** code for this rule:
+
+```js
+const [color, setColor] = React.useState();


Would someone want to configure "set" to make "update" allowed? Should we allow that to be configurable?

Have you seen this a lot before? I haven't seen this pattern before.

I suppose allowing arbitrary configuration of the prefix would be ok though (eg. a setterPrefix option?) - would allow for flexibility for some teams.

Would someone want to configure "set" to make "update" allowed? Should we allow that to be configurable?

Just as the naming of hooks is already opinionated, useXYZ, all the React docs consistently use [thing, setThing].
I think it's best to lean on this convention here.

lib/rules/hook-use-state-names.js

ljharb · 2020-12-30T03:30:50Z

tests/lib/rules/hook-use-state-names.js

+      code: 'const useStateResult = useState()'
+    },
+    {
+      code: 'const [, , unused] = useState()'


this seems like it should be an error, but i guess not from this rule

Yeah, in my workflow this issue would be caught by the type system.
This test is intended to indicate the rule's non-opinion around a third argument (and to make sure I didn't throw an error on weird inputs)

This is now an error. (not fixable)

ljharb · 2020-12-30T03:31:47Z

tests/lib/rules/hook-use-state-names.js

+      code: 'const [, setColor] = useState()'
+    },
+    {
+      code: 'const useStateResult = useState()'


what about if someone does const [state, setThatState] = useStateResult; or const state = useStateResult[0]; const setThatState = useStateResult[1]; on the next lines?

This rule only looks for calls to useState which are directly destructured+assigned;

These intermediate-value forms aren't recognized by this rule.

This is now an error as well. (not fixable)

ljharb · 2020-12-30T03:32:33Z

tests/lib/rules/hook-use-state-names.js

+    {
+      code: 'const [color, setColor] = useState<string>()',
+      parser: parsers.TYPESCRIPT_ESLINT
+    },
+    {
+      code: 'const [color, setColor] = useState<string>(\'#ffffff\')',
+      parser: parsers.TYPESCRIPT_ESLINT
+    }


let's also duplicate these and add parsers['@TYPESCRIPT_ESLINT'] so we can handle the new parser too. grep for parsers.TS( to find examples of how to do this safely.

I think I added these correctly.

ljharb · 2020-12-30T03:33:13Z

tests/lib/rules/hook-use-state-names.js

+        message: 'setState variable names are not symmetric, setter should be setColor'
+      }],
+      output: 'const [color, setColor] = useState<string>()',
+      parser: parsers.TYPESCRIPT_ESLINT


duplicate test case with the new parser here too

Ensure two symmetrically-named variables are destructured from useState hook calls

duncanbeevers · 2021-01-04T01:37:53Z

@ljharb The scope of this rule has increased, and its naming has been changed from hook-use-state-names to the "generic" hook-use-state.

As you mentioned in your first review, certain unusual usages of setState weren't addressed by this rule.
In addressing these issues the other comments raised in this PR, the prescriptive nature of this rule seemed clearer.

Towards that end of providing gentle, but clear guidance in the usage of this hook+rule, the naming has been changed to hook-use-state.

It is now a bit more prescriptive, and therefore a bit more helpful in its fix suggestions.

Stuff that didn't change

there is no configuration
all previously-invalid cases remain invalid (invalid cases were focused solely on variable naming violations)

Stuff that changed

enforced destructuring of useState call to value+setter
added fix for mis-destructured assigments when value variable name is present
- setter variable name is inferred from value variable name
- over+under arity destructures, and mis-named destructures are all fixed by replacement with canonicalized 2-arity form
added tests for pathological cases

// Who does this!?
const [] = useState();
const [, , , , ] = useState();

duncanbeevers · 2021-02-10T16:30:10Z

Closing now that the core rule change has landed.
eslint/eslint#14012

duncanbeevers · 2021-02-10T16:34:16Z

Oops wrong PR. I'll re-open this one and remove the no-unused-expresssion PR.

mea culpa

duncanbeevers changed the title ~~[New] : symmetric names~~ [New] : Symmetric setState hook variable names Dec 9, 2020

karlhorky mentioned this pull request Dec 9, 2020

Feature request: enforce naming convention for useState() #2417

Closed

duncanbeevers force-pushed the hook-set-state-names branch 5 times, most recently from fd0f854 to b244528 Compare December 11, 2020 21:31

duncanbeevers changed the title ~~[New] : Symmetric setState hook variable names~~ [New] : Symmetric useState hook variable names Dec 12, 2020

duncanbeevers force-pushed the hook-set-state-names branch 3 times, most recently from 52df451 to 355ce77 Compare December 15, 2020 22:37

ljharb requested changes Dec 30, 2020

View reviewed changes

ljharb marked this pull request as draft December 30, 2020 04:14

duncanbeevers marked this pull request as ready for review December 30, 2020 21:20

duncanbeevers requested a review from ljharb December 30, 2020 23:08

[New] : Symmetric useState hook variable names

80ed3fc

Ensure two symmetrically-named variables are destructured from useState hook calls

duncanbeevers force-pushed the hook-set-state-names branch from dea7de1 to 80ed3fc Compare January 4, 2021 01:27

duncanbeevers changed the title ~~[New] : Symmetric useState hook variable names~~ [New] : Enforce useState hook destructuring and variable naming Jan 4, 2021

duncanbeevers added 4 commits January 4, 2021 06:24

fixup! [New] : Symmetric useState hook variable names

619d7df

fixup! [New] : Symmetric useState hook variable names

a9dc68c

fixup! [New] : Symmetric useState hook variable names

283d017

fixup! [New] : Symmetric useState hook variable names

df0c575

duncanbeevers closed this Feb 10, 2021

duncanbeevers deleted the hook-set-state-names branch February 10, 2021 16:30

duncanbeevers restored the hook-set-state-names branch February 10, 2021 16:33

ljharb added the eslint label Feb 10, 2021

duncanbeevers mentioned this pull request Feb 10, 2021

[New] Symmetric useState hook variable names #2921

Merged

ljharb removed the eslint label Feb 10, 2021

Uh oh!

[New] : Enforce useState hook destructuring and variable naming #2873

[New] : Enforce useState hook destructuring and variable naming #2873

Uh oh!

Conversation

duncanbeevers commented Dec 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

karlhorky commented Dec 9, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

duncanbeevers Jan 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

duncanbeevers Jan 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

duncanbeevers commented Jan 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Stuff that didn't change

Stuff that changed

Uh oh!

duncanbeevers commented Feb 10, 2021

Uh oh!

duncanbeevers commented Feb 10, 2021

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

duncanbeevers commented Dec 9, 2020 •

edited

Loading

duncanbeevers Jan 4, 2021 •

edited

Loading

duncanbeevers Jan 4, 2021 •

edited

Loading

duncanbeevers commented Jan 4, 2021 •

edited

Loading