Skip to content

Commit c137dd6

Browse files
authored
Fix exhaustive deps bug with flow type casting. (facebook#35691)
Summary: I noticed there's a bug where the lint will recognize the type on a cast annotation as a missing dependency; ``` function MyComponent() { type ColumnKey = 'id' | 'name'; type Item = {id: string, name: string}; const columns = useMemo( () => [ { type: 'text', key: 'id', } as TextColumn<ColumnKey, Item>, ^^^^^^^^ here ], [], ); } ``` This is due to the AST of AsExpressions being something like: AsExpression └── typeAnnotation: GenericTypeAnnotation └── typeParameters: TypeParameterInstantiation └── params[0]: GenericTypeAnnotation └── id: Identifier (name: "ColumnKey") Where `ColumnKey` never has a TypeParameter Annotation. So we need to consider it to be a flow type due to it belonging to a GenericTypeAnnotation Test Plan: Added unit tests Before: ``` Test Suites: 1 failed, 2 passed, 3 total Tests: 2 failed, 5065 passed, 5067 total Snapshots: 0 total Time: 16.517 s Ran all test suites. error Command failed with exit code 1. ``` After: ``` PASS __tests__/ReactCompilerRuleTypescript-test.ts PASS __tests__/ESLintRulesOfHooks-test.js (6.192 s) PASS __tests__/ESLintRuleExhaustiveDeps-test.js (9.97 s) Test Suites: 3 passed, 3 total Tests: 5067 passed, 5067 total Snapshots: 0 total Time: 10.21 s, estimated 11 s Ran all test suites. ✨ Done in 12.66s. ```
1 parent 22a20e1 commit c137dd6

2 files changed

Lines changed: 26 additions & 4 deletions

File tree

packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7913,6 +7913,25 @@ const testsFlow = {
79137913
}
79147914
`,
79157915
},
7916+
// Flow type aliases in type assertions should not be flagged as missing dependencies
7917+
{
7918+
code: normalizeIndent`
7919+
function MyComponent() {
7920+
type ColumnKey = 'id' | 'name';
7921+
type Item = {id: string, name: string};
7922+
7923+
const columns = useMemo(
7924+
() => [
7925+
{
7926+
type: 'text',
7927+
key: 'id',
7928+
} as TextColumn<ColumnKey, Item>,
7929+
],
7930+
[],
7931+
);
7932+
}
7933+
`,
7934+
},
79167935
],
79177936
invalid: [
79187937
{

packages/eslint-plugin-react-hooks/src/rules/ExhaustiveDeps.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import type {
2121
VariableDeclarator,
2222
} from 'estree';
2323

24-
import { getAdditionalEffectHooksFromSettings } from '../shared/Utils';
24+
import {getAdditionalEffectHooksFromSettings} from '../shared/Utils';
2525

2626
type DeclaredDependency = {
2727
key: string;
@@ -80,7 +80,6 @@ const rule = {
8080
const rawOptions = context.options && context.options[0];
8181
const settings = context.settings || {};
8282

83-
8483
// Parse the `additionalHooks` regex.
8584
// Use rule-level additionalHooks if provided, otherwise fall back to settings
8685
const additionalHooks =
@@ -565,8 +564,12 @@ const rule = {
565564
continue;
566565
}
567566
// Ignore Flow type parameters
568-
// @ts-expect-error We don't have flow types
569-
if (def.type === 'TypeParameter') {
567+
if (
568+
// @ts-expect-error We don't have flow types
569+
def.type === 'TypeParameter' ||
570+
// @ts-expect-error Flow-specific AST node type
571+
dependencyNode.parent?.type === 'GenericTypeAnnotation'
572+
) {
570573
continue;
571574
}
572575

0 commit comments

Comments
 (0)