Consider components in jsx as missing dependencies in @typescript-eslint/parser@4.x (#19815)
* Run JS tests with TS esling parser * Add failing test * fix: Mark JSXIdentifier has missing dependency * Safe isSameIdentifier
Sebastian Silbermann committed
Sep 11, 2020 at 14:13 UTC
0f70d4dd667d8c953aaf8b0d40f6a0439cd4ab27
2 files changed
+116
-39
packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js
+113
-37
@@ -31,6 +31,7 @@ function normalizeIndent(strings) {
31
// }
32
// ***************************************************
33
34
+// Tests that are valid/invalid across all parsers
35
const tests = {
36
valid: [
37
{
@@ -1322,16 +1323,6 @@ const tests = {
1323
}
1324
`,
1325
},
1325
- // Ignore Generic Type Variables for arrow functions
1326
- {
1327
- code: normalizeIndent`
1328
- function Example({ prop }) {
1329
- const bar = useEffect(<T>(a: T): Hello => {
1330
- prop();
1331
- }, [prop]);
1332
- }
1333
- `,
1334
- },
1326
// Ignore arguments keyword for arrow functions.
1327
{
1328
code: normalizeIndent`
@@ -7466,24 +7457,7 @@ const tests = {
7457
},
7458
],
7459
},
7469
- {
7470
- code: normalizeIndent`
7471
- function Foo() {
7472
- const foo = ({}: any);
7473
- useMemo(() => {
7474
- console.log(foo);
7475
- }, [foo]);
7476
- }
7477
- `,
7478
- errors: [
7479
- {
7480
- message:
7481
- "The 'foo' object makes the dependencies of useMemo Hook (at line 6) change on every render. " +
7482
- "Move it inside the useMemo callback. Alternatively, wrap the initialization of 'foo' in its own useMemo() Hook.",
7483
- suggestions: undefined,
7484
- },
7485
- ],
7486
- },
7460
+
7461
{
7462
code: normalizeIndent`
7463
function Foo() {
@@ -7532,6 +7506,43 @@ const tests = {
7506
],
7507
};
7508
7509
+// Tests that are only valid/invalid across parsers supporting Flow
7510
+const testsFlow = {
7511
+ valid: [
7512
+ // Ignore Generic Type Variables for arrow functions
7513
+ {
7514
+ code: normalizeIndent`
7515
+ function Example({ prop }) {
7516
+ const bar = useEffect(<T>(a: T): Hello => {
7517
+ prop();
7518
+ }, [prop]);
7519
+ }
7520
+ `,
7521
+ },
7522
+ ],
7523
+ invalid: [
7524
+ {
7525
+ code: normalizeIndent`
7526
+ function Foo() {
7527
+ const foo = ({}: any);
7528
+ useMemo(() => {
7529
+ console.log(foo);
7530
+ }, [foo]);
7531
+ }
7532
+ `,
7533
+ errors: [
7534
+ {
7535
+ message:
7536
+ "The 'foo' object makes the dependencies of useMemo Hook (at line 6) change on every render. " +
7537
+ "Move it inside the useMemo callback. Alternatively, wrap the initialization of 'foo' in its own useMemo() Hook.",
7538
+ suggestions: undefined,
7539
+ },
7540
+ ],
7541
+ },
7542
+ ],
7543
+};
7544
+
7545
+// Tests that are only valid/invalid across parsers supporting TypeScript
7546
const testsTypescript = {
7547
valid: [
7548
{
@@ -7928,6 +7939,43 @@ const testsTypescript = {
7939
],
7940
};
7941
7942
+// Tests that are only valid/invalid for `@typescript-eslint/parser@4.x`
7943
+const testsTypescriptEslintParserV4 = {
7944
+ valid: [],
7945
+ invalid: [
7946
+ // TODO: Should also be invalid as part of the JS test suite i.e. be invalid with babel eslint parsers.
7947
+ // It doesn't use any explicit types but any JS is still valid TS.
7948
+ {
7949
+ code: normalizeIndent`
7950
+ function Foo({ Component }) {
7951
+ React.useEffect(() => {
7952
+ console.log(<Component />);
7953
+ }, []);
7954
+ };
7955
+ `,
7956
+ errors: [
7957
+ {
7958
+ message:
7959
+ "React Hook React.useEffect has a missing dependency: 'Component'. " +
7960
+ 'Either include it or remove the dependency array.',
7961
+ suggestions: [
7962
+ {
7963
+ desc: 'Update the dependencies array to be: [Component]',
7964
+ output: normalizeIndent`
7965
+ function Foo({ Component }) {
7966
+ React.useEffect(() => {
7967
+ console.log(<Component />);
7968
+ }, [Component]);
7969
+ };
7970
+ `,
7971
+ },
7972
+ ],
7973
+ },
7974
+ ],
7975
+ },
7976
+ ],
7977
+};
7978
+
7979
// For easier local testing
7980
if (!process.env.CI) {
7981
let only = [];
@@ -7935,8 +7983,12 @@ if (!process.env.CI) {
7983
[
7984
...tests.valid,
7985
...tests.invalid,
7986
+ ...testsFlow.valid,
7987
+ ...testsFlow.invalid,
7988
...testsTypescript.valid,
7989
...testsTypescript.invalid,
7990
+ ...testsTypescriptEslintParserV4.valid,
7991
+ ...testsTypescriptEslintParserV4.invalid,
7992
].forEach(t => {
7993
if (t.skip) {
7994
delete t.skip;
@@ -7958,25 +8010,44 @@ if (!process.env.CI) {
8010
};
8011
tests.valid = tests.valid.filter(predicate);
8012
tests.invalid = tests.invalid.filter(predicate);
8013
+ testsFlow.valid = testsFlow.valid.filter(predicate);
8014
+ testsFlow.invalid = testsFlow.invalid.filter(predicate);
8015
testsTypescript.valid = testsTypescript.valid.filter(predicate);
8016
testsTypescript.invalid = testsTypescript.invalid.filter(predicate);
8017
}
8018
8019
describe('react-hooks', () => {
8020
const parserOptions = {
8021
+ ecmaFeatures: {
8022
+ jsx: true,
8023
+ },
8024
ecmaVersion: 6,
8025
sourceType: 'module',
8026
};
8027
8028
+ const testsBabelEslint = {
8029
+ valid: [...testsFlow.valid, ...tests.valid],
8030
+ invalid: [...testsFlow.invalid, ...tests.invalid],
8031
+ };
8032
+
8033
new ESLintTester({
8034
parser: require.resolve('babel-eslint'),
8035
parserOptions,
7974
- }).run('parser: babel-eslint', ReactHooksESLintRule, tests);
8036
+ }).run('parser: babel-eslint', ReactHooksESLintRule, testsBabelEslint);
8037
8038
new ESLintTester({
8039
parser: require.resolve('@babel/eslint-parser'),
8040
parserOptions,
7979
- }).run('parser: @babel/eslint-parser', ReactHooksESLintRule, tests);
8041
+ }).run(
8042
+ 'parser: @babel/eslint-parser',
8043
+ ReactHooksESLintRule,
8044
+ testsBabelEslint
8045
+ );
8046
+
8047
+ const testsTypescriptEslintParser = {
8048
+ valid: [...testsTypescript.valid, ...tests.valid],
8049
+ invalid: [...testsTypescript.invalid, ...tests.invalid],
8050
+ };
8051
8052
new ESLintTester({
8053
parser: require.resolve('@typescript-eslint/parser-v2'),
@@ -7984,7 +8055,7 @@ describe('react-hooks', () => {
8055
}).run(
8056
'parser: @typescript-eslint/parser@2.x',
8057
ReactHooksESLintRule,
7987
- testsTypescript
8058
+ testsTypescriptEslintParser
8059
);
8060
8061
new ESLintTester({
@@ -7993,15 +8064,20 @@ describe('react-hooks', () => {
8064
}).run(
8065
'parser: @typescript-eslint/parser@3.x',
8066
ReactHooksESLintRule,
7996
- testsTypescript
8067
+ testsTypescriptEslintParser
8068
);
8069
8070
new ESLintTester({
8071
parser: require.resolve('@typescript-eslint/parser-v4'),
8072
parserOptions,
8002
- }).run(
8003
- 'parser: @typescript-eslint/parser@4.x',
8004
- ReactHooksESLintRule,
8005
- testsTypescript
8006
- );
8073
+ }).run('parser: @typescript-eslint/parser@4.x', ReactHooksESLintRule, {
8074
+ valid: [
8075
+ ...testsTypescriptEslintParserV4.valid,
8076
+ ...testsTypescriptEslintParser.valid,
8077
+ ],
8078
+ invalid: [
8079
+ ...testsTypescriptEslintParserV4.invalid,
8080
+ ...testsTypescriptEslintParser.invalid,
8081
+ ],
8082
+ });
8083
});
packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js
+3
-2
@@ -1622,7 +1622,7 @@ function markNode(node, optionalChains, result) {
1622
* Otherwise throw.
1623
*/
1624
function analyzePropertyChain(node, optionalChains) {
1625
- if (node.type === 'Identifier') {
1625
+ if (node.type === 'Identifier' || node.type === 'JSXIdentifier') {
1626
const result = node.name;
1627
if (optionalChains) {
1628
// Mark as required.
@@ -1779,7 +1779,8 @@ function isNodeLike(val) {
1779
1780
function isSameIdentifier(a, b) {
1781
return (
1782
- a.type === 'Identifier' &&
1782
+ (a.type === 'Identifier' || a.type === 'JSXIdentifier') &&
1783
+ a.type === b.type &&
1784
a.name === b.name &&
1785
a.range[0] === b.range[0] &&
1786
a.range[1] === b.range[1]