diff --git a/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js index 36b5d646cebb8..0c782dc25ae02 100644 --- a/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js +++ b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js @@ -31,6 +31,7 @@ function normalizeIndent(strings) { // } // *************************************************** +// Tests that are valid/invalid across all parsers const tests = { valid: [ { @@ -1322,16 +1323,6 @@ const tests = { } `, }, - // Ignore Generic Type Variables for arrow functions - { - code: normalizeIndent` - function Example({ prop }) { - const bar = useEffect((a: T): Hello => { - prop(); - }, [prop]); - } - `, - }, // Ignore arguments keyword for arrow functions. { code: normalizeIndent` @@ -7466,24 +7457,7 @@ const tests = { }, ], }, - { - code: normalizeIndent` - function Foo() { - const foo = ({}: any); - useMemo(() => { - console.log(foo); - }, [foo]); - } - `, - errors: [ - { - message: - "The 'foo' object makes the dependencies of useMemo Hook (at line 6) change on every render. " + - "Move it inside the useMemo callback. Alternatively, wrap the initialization of 'foo' in its own useMemo() Hook.", - suggestions: undefined, - }, - ], - }, + { code: normalizeIndent` function Foo() { @@ -7532,6 +7506,43 @@ const tests = { ], }; +// Tests that are only valid/invalid across parsers supporting Flow +const testsFlow = { + valid: [ + // Ignore Generic Type Variables for arrow functions + { + code: normalizeIndent` + function Example({ prop }) { + const bar = useEffect((a: T): Hello => { + prop(); + }, [prop]); + } + `, + }, + ], + invalid: [ + { + code: normalizeIndent` + function Foo() { + const foo = ({}: any); + useMemo(() => { + console.log(foo); + }, [foo]); + } + `, + errors: [ + { + message: + "The 'foo' object makes the dependencies of useMemo Hook (at line 6) change on every render. " + + "Move it inside the useMemo callback. Alternatively, wrap the initialization of 'foo' in its own useMemo() Hook.", + suggestions: undefined, + }, + ], + }, + ], +}; + +// Tests that are only valid/invalid across parsers supporting TypeScript const testsTypescript = { valid: [ { @@ -7928,6 +7939,43 @@ const testsTypescript = { ], }; +// Tests that are only valid/invalid for `@typescript-eslint/parser@4.x` +const testsTypescriptEslintParserV4 = { + valid: [], + invalid: [ + // TODO: Should also be invalid as part of the JS test suite i.e. be invalid with babel eslint parsers. + // It doesn't use any explicit types but any JS is still valid TS. + { + code: normalizeIndent` + function Foo({ Component }) { + React.useEffect(() => { + console.log(); + }, []); + }; + `, + errors: [ + { + message: + "React Hook React.useEffect has a missing dependency: 'Component'. " + + 'Either include it or remove the dependency array.', + suggestions: [ + { + desc: 'Update the dependencies array to be: [Component]', + output: normalizeIndent` + function Foo({ Component }) { + React.useEffect(() => { + console.log(); + }, [Component]); + }; + `, + }, + ], + }, + ], + }, + ], +}; + // For easier local testing if (!process.env.CI) { let only = []; @@ -7935,8 +7983,12 @@ if (!process.env.CI) { [ ...tests.valid, ...tests.invalid, + ...testsFlow.valid, + ...testsFlow.invalid, ...testsTypescript.valid, ...testsTypescript.invalid, + ...testsTypescriptEslintParserV4.valid, + ...testsTypescriptEslintParserV4.invalid, ].forEach(t => { if (t.skip) { delete t.skip; @@ -7958,25 +8010,44 @@ if (!process.env.CI) { }; tests.valid = tests.valid.filter(predicate); tests.invalid = tests.invalid.filter(predicate); + testsFlow.valid = testsFlow.valid.filter(predicate); + testsFlow.invalid = testsFlow.invalid.filter(predicate); testsTypescript.valid = testsTypescript.valid.filter(predicate); testsTypescript.invalid = testsTypescript.invalid.filter(predicate); } describe('react-hooks', () => { const parserOptions = { + ecmaFeatures: { + jsx: true, + }, ecmaVersion: 6, sourceType: 'module', }; + const testsBabelEslint = { + valid: [...testsFlow.valid, ...tests.valid], + invalid: [...testsFlow.invalid, ...tests.invalid], + }; + new ESLintTester({ parser: require.resolve('babel-eslint'), parserOptions, - }).run('parser: babel-eslint', ReactHooksESLintRule, tests); + }).run('parser: babel-eslint', ReactHooksESLintRule, testsBabelEslint); new ESLintTester({ parser: require.resolve('@babel/eslint-parser'), parserOptions, - }).run('parser: @babel/eslint-parser', ReactHooksESLintRule, tests); + }).run( + 'parser: @babel/eslint-parser', + ReactHooksESLintRule, + testsBabelEslint + ); + + const testsTypescriptEslintParser = { + valid: [...testsTypescript.valid, ...tests.valid], + invalid: [...testsTypescript.invalid, ...tests.invalid], + }; new ESLintTester({ parser: require.resolve('@typescript-eslint/parser-v2'), @@ -7984,7 +8055,7 @@ describe('react-hooks', () => { }).run( 'parser: @typescript-eslint/parser@2.x', ReactHooksESLintRule, - testsTypescript + testsTypescriptEslintParser ); new ESLintTester({ @@ -7993,15 +8064,20 @@ describe('react-hooks', () => { }).run( 'parser: @typescript-eslint/parser@3.x', ReactHooksESLintRule, - testsTypescript + testsTypescriptEslintParser ); new ESLintTester({ parser: require.resolve('@typescript-eslint/parser-v4'), parserOptions, - }).run( - 'parser: @typescript-eslint/parser@4.x', - ReactHooksESLintRule, - testsTypescript - ); + }).run('parser: @typescript-eslint/parser@4.x', ReactHooksESLintRule, { + valid: [ + ...testsTypescriptEslintParserV4.valid, + ...testsTypescriptEslintParser.valid, + ], + invalid: [ + ...testsTypescriptEslintParserV4.invalid, + ...testsTypescriptEslintParser.invalid, + ], + }); }); diff --git a/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js b/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js index d80d701c6869b..7c970f7c8f6ac 100644 --- a/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js +++ b/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js @@ -1622,7 +1622,7 @@ function markNode(node, optionalChains, result) { * Otherwise throw. */ function analyzePropertyChain(node, optionalChains) { - if (node.type === 'Identifier') { + if (node.type === 'Identifier' || node.type === 'JSXIdentifier') { const result = node.name; if (optionalChains) { // Mark as required. @@ -1779,7 +1779,8 @@ function isNodeLike(val) { function isSameIdentifier(a, b) { return ( - a.type === 'Identifier' && + (a.type === 'Identifier' || a.type === 'JSXIdentifier') && + a.type === b.type && a.name === b.name && a.range[0] === b.range[0] && a.range[1] === b.range[1]