[Fix] destructuring-assignment: Handle destructuring of useContext in SFC
#2797
Conversation
…in SFC Co-authored-by: Jin Young Park <jy2045@sogang.ac.kr> Co-authored-by: Jordan Harband <ljharb@gmail.com>
| const destructuringUseContext = destructuring && node.init.callee && node.init.callee.name === 'useContext'; | ||
| // let foo = useContext(aContext); | ||
| const assignUseContext = identifier && node.init.callee && node.init.callee.name === 'useContext'; |
There was a problem hiding this comment.
are we sure useContext is the react hook? I think we should make sure that nobody's using a custom hook or function named useContext, and that this useContext is imported/required from react.
There was a problem hiding this comment.
https://reactjs.org/docs/hooks-reference.html#usecontext
here in official docs of react says useContext is one of basic hooks with useState, useEffect.
and we import useContext like import {useContext} from 'react';
I think if somebody's using function named useContext which overwrite basic hooks , I think we should make another rule about custom hooks which overwrite basic hooks.
There was a problem hiding this comment.
Someone might be using a version of React that doesn't have hooks.
There was a problem hiding this comment.
then is there any possible way to check version of react? or we should not include this part.
There was a problem hiding this comment.
Yes! Check out lib/util/version.js's testReactVersion method.
There was a problem hiding this comment.
do I have to import testReactVersion method in lib/rules/destructuring-assignment' to check react version in rules? I have no idea.. could you help me with this?
|
@Zinyon are you still interested in completing this PR? |
|
The |
|
@ljharb sure, i will look around |
ljharb
force-pushed
the
master
branch
6 times, most recently
from
59af733
to
865ed16
Compare
ljharb
force-pushed
the
master
branch
4 times, most recently
from
069314a
to
181c68f
Compare
Codecov Report
@@ Coverage Diff @@
## master #2797 +/- ##
=======================================
Coverage 97.58% 97.58%
=======================================
Files 130 130
Lines 9223 9223
Branches 3349 3349
=======================================
Hits 9000 9000
Misses 223 223 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
|
I went ahead and added the react version dependence; i'll leave "make sure it's destructured from the pragma" as a followup. |
…Context in SFC" - [Tests] `destructuring-assignment`: Add more modern context cases This reverts commit 523db20 / jsx-eslint#2797 Fixes jsx-eslint#3536.
i reopen PR on resolve issue - #2309 cause i did a lot of commits while editing.
Fixes #2309. Closes #2787.