aet/eslint-rules
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Upgrade to ESLint 9

Browse Source
This commit is contained in:
Alex
2024-10-16 00:29:26 -04:00
parent 0138cabb27
commit 00d0dfa107
53 changed files with 3813 additions and 2207 deletions
Download Patch File Download Diff File Expand all files Collapse all files

253
packages/eslint-plugin-react-hooks/ExhaustiveDeps.ts
View File

@ -22,8 +22,11 @@ import type {
ChainExpression,
Pattern,
OptionalMemberExpression,
ArrayExpression,
VariableDeclaration,
} from 'estree';
import type { FromSchema } from 'json-schema-to-ts';
import { __EXPERIMENTAL__ } from './index';
const schema = {
@ -81,7 +84,23 @@ const rule: Rule.RuleModule = {
context.report(problem);
}
const scopeManager = context.sourceCode.scopeManager;
/**
* SourceCode#getText that also works down to ESLint 3.0.0
*/
const getSource =
typeof context.getSource === 'function'
? (node: Node) => context.getSource(node)
: (node: Node) => context.sourceCode.getText(node);
/**
* SourceCode#getScope that also works down to ESLint 3.0.0
*/
const getScope =
typeof context.getScope === 'function'
? () => context.getScope()
: (node: Node) => context.sourceCode.getScope(node);
const scopeManager = context.getSourceCode().scopeManager;
// Should be shared between visitors.
const setStateCallSites = new WeakMap<Expression, Pattern>();
@ -128,7 +147,7 @@ const rule: Rule.RuleModule = {
' }\n' +
' fetchData();\n' +
`}, [someId]); // Or [] if effect doesn't need props or state\n\n` +
'Learn more about data fetching with Hooks: https://reactjs.org/link/hooks-data-fetching',
'Learn more about data fetching with Hooks: https://react.dev/link/hooks-data-fetching',
});
}
@ -173,6 +192,8 @@ const rule: Rule.RuleModule = {
// ^^^ true for this reference
// const [state, dispatch] = useReducer() / React.useReducer()
// ^^^ true for this reference
// const [state, dispatch] = useActionState() / React.useActionState()
// ^^^ true for this reference
// const ref = useRef()
// ^^^ true for this reference
// const onStuff = useEffectEvent(() => {})
@ -187,31 +208,32 @@ const rule: Rule.RuleModule = {
return false;
}
// Look for `let stuff = ...`
if (def.node.type !== 'VariableDeclarator') {
const node = def.node as Node;
if (node.type !== 'VariableDeclarator') {
return false;
}
let init = (def.node as VariableDeclarator).init;
let init = node.init;
if (init == null) {
return false;
}
while (init.type === 'TSAsExpression') {
while (init.type === 'TSAsExpression' || init.type === 'AsExpression') {
init = init.expression;
}
// Detect primitive constants
// const foo = 42
let declaration = def.node.parent;
let declaration = node.parent;
if (declaration == null) {
// This might happen if variable is declared after the callback.
// In that case ESLint won't set up .parent refs.
// So we'll set them up manually.
fastFindReferenceWithParent(componentScope.block, def.node.id);
declaration = def.node.parent;
fastFindReferenceWithParent(componentScope.block, node.id);
declaration = node.parent;
if (declaration == null) {
return false;
}
}
if (
declaration.kind === 'const' &&
(declaration as VariableDeclaration).kind === 'const' &&
init.type === 'Literal' &&
(typeof init.value === 'string' ||
typeof init.value === 'number' ||
@ -252,7 +274,11 @@ const rule: Rule.RuleModule = {
}
// useEffectEvent() return value is always unstable.
return true;
} else if (name === 'useState' || name === 'useReducer') {
} else if (
name === 'useState' ||
name === 'useReducer' ||
name === 'useActionState'
) {
// Only consider second value in initializing tuple stable.
if (
id.type === 'ArrayPattern' &&
@ -264,14 +290,14 @@ const rule: Rule.RuleModule = {
if (name === 'useState') {
const references = resolved.references;
let writeCount = 0;
for (let i = 0; i < references.length; i++) {
if (references[i].isWrite()) {
for (const reference of references) {
if (reference.isWrite()) {
writeCount++;
}
if (writeCount > 1) {
return false;
}
setStateCallSites.set(references[i].identifier, id.elements[0]!);
setStateCallSites.set(reference.identifier, id.elements[0]!);
}
}
// Setter is stable.
@ -279,27 +305,25 @@ const rule: Rule.RuleModule = {
} else if (id.elements[0] === resolved.identifiers[0]) {
if (name === 'useState') {
const references = resolved.references;
for (let i = 0; i < references.length; i++) {
stateVariables.add(references[i].identifier);
for (const reference of references) {
stateVariables.add(reference.identifier);
}
}
// State variable itself is dynamic.
return false;
}
}
} else if (name === 'useTransition') {
} else if (
// Only consider second value in initializing tuple stable.
if (
id.type === 'ArrayPattern' &&
id.elements.length === 2 &&
Array.isArray(resolved.identifiers)
) {
// Is second tuple value the same reference we're checking?
if (id.elements[1] === resolved.identifiers[0]) {
// Setter is stable.
return true;
}
}
name === 'useTransition' &&
id.type === 'ArrayPattern' &&
id.elements.length === 2 &&
Array.isArray(resolved.identifiers) &&
// Is second tuple value the same reference we're checking?
id.elements[1] === resolved.identifiers[0]
) {
// Setter is stable.
return true;
}
// By default assume it's dynamic.
return false;
@ -319,7 +343,7 @@ const rule: Rule.RuleModule = {
}
// Search the direct component subscopes for
// top-level function definitions matching this reference.
const fnNode = def.node;
const fnNode = def.node as Node;
const childScopes = componentScope.childScopes;
let fnScope = null;
let i;
@ -424,9 +448,9 @@ const rule: Rule.RuleModule = {
dependencyNode.type === 'Identifier' &&
(dependencyNode.parent!.type === 'MemberExpression' ||
dependencyNode.parent!.type === 'OptionalMemberExpression') &&
!dependencyNode.parent!.computed &&
dependencyNode.parent!.property.type === 'Identifier' &&
dependencyNode.parent!.property.name === 'current' &&
!dependencyNode.parent.computed &&
dependencyNode.parent.property.type === 'Identifier' &&
dependencyNode.parent.property.name === 'current' &&
// ...in a cleanup function or below...
isInsideEffectCleanup(reference)
) {
@ -479,12 +503,11 @@ const rule: Rule.RuleModule = {
// Warn about accessing .current in cleanup effects.
currentRefsInEffectCleanup.forEach(({ reference, dependencyNode }, dependency) => {
const references: Scope.Reference[] = reference.resolved!.references;
const references: Scope.Reference[] = reference.resolved.references;
// Is React managing this ref or us?
// Let's see if we can find a .current assignment.
let foundCurrentAssignment = false;
for (let i = 0; i < references.length; i++) {
const { identifier } = references[i];
for (const { identifier } of references) {
const { parent } = identifier;
if (
parent != null &&
@ -496,7 +519,7 @@ const rule: Rule.RuleModule = {
parent.property.name === 'current' &&
// ref.current = <something>
parent.parent!.type === 'AssignmentExpression' &&
parent.parent!.left === parent
parent.parent.left === parent
) {
foundCurrentAssignment = true;
break;
@ -529,11 +552,11 @@ const rule: Rule.RuleModule = {
node: writeExpr,
message:
`Assignments to the '${key}' variable from inside React Hook ` +
`${context.getSource(reactiveHook)} will be lost after each ` +
`${getSource(reactiveHook)} will be lost after each ` +
`render. To preserve the value over time, store it in a useRef ` +
`Hook and keep the mutable value in the '.current' property. ` +
`Otherwise, you can move this variable directly inside ` +
`${context.getSource(reactiveHook)}.`,
`${getSource(reactiveHook)}.`,
});
}
@ -543,11 +566,11 @@ const rule: Rule.RuleModule = {
if (isStable) {
stableDependencies.add(key);
}
references.forEach(reference => {
for (const reference of references) {
if (reference.writeExpr) {
reportStaleAssignment(reference.writeExpr, key);
}
});
}
});
if (staleAssignments.size > 0) {
@ -563,15 +586,15 @@ const rule: Rule.RuleModule = {
if (setStateInsideEffectWithoutDeps) {
return;
}
references.forEach(reference => {
for (const reference of references) {
if (setStateInsideEffectWithoutDeps) {
return;
continue;
}
const id = reference.identifier;
const isSetState: boolean = setStateCallSites.has(id);
if (!isSetState) {
return;
continue;
}
let fnScope: Scope.Scope = reference.from;
@ -583,9 +606,8 @@ const rule: Rule.RuleModule = {
// TODO: we could potentially ignore early returns.
setStateInsideEffectWithoutDeps = key;
}
});
}
});
if (setStateInsideEffectWithoutDeps) {
const { suggestedDependencies } = collectRecommendations({
dependencies,
@ -620,49 +642,56 @@ const rule: Rule.RuleModule = {
const declaredDependencies: DeclaredDependency[] = [];
const externalDependencies = new Set<string>();
if (declaredDependenciesNode.type !== 'ArrayExpression') {
const isArrayExpression = declaredDependenciesNode.type === 'ArrayExpression';
const isTSAsArrayExpression =
declaredDependenciesNode.type === 'TSAsExpression' &&
declaredDependenciesNode.expression.type === 'ArrayExpression';
if (!isArrayExpression && !isTSAsArrayExpression) {
// If the declared dependencies are not an array expression then we
// can't verify that the user provided the correct dependencies. Tell
// the user this in an error.
reportProblem({
node: declaredDependenciesNode,
message:
`React Hook ${context.getSource(reactiveHook)} was passed a ` +
`React Hook ${getSource(reactiveHook)} was passed a ` +
'dependency list that is not an array literal. This means we ' +
"can't statically verify whether you've passed the correct " +
'dependencies.',
});
} else {
declaredDependenciesNode.elements.forEach(declaredDependencyNode => {
const arrayExpression = isTSAsArrayExpression
? declaredDependenciesNode.expression
: declaredDependenciesNode;
for (const declaredDependencyNode of (arrayExpression as ArrayExpression)
.elements) {
// Skip elided elements.
if (declaredDependencyNode === null) {
return;
continue;
}
// If we see a spread element then add a special warning.
if (declaredDependencyNode.type === 'SpreadElement') {
reportProblem({
node: declaredDependencyNode,
message:
`React Hook ${context.getSource(reactiveHook)} has a spread ` +
`React Hook ${getSource(reactiveHook)} has a spread ` +
"element in its dependency array. This means we can't " +
"statically verify whether you've passed the " +
'correct dependencies.',
});
return;
continue;
}
if (useEffectEventVariables.has(declaredDependencyNode)) {
reportProblem({
node: declaredDependencyNode,
message:
'Functions returned from `useEffectEvent` must not be included in the dependency array. ' +
`Remove \`${context.getSource(declaredDependencyNode)}\` from the list.`,
`Remove \`${getSource(declaredDependencyNode)}\` from the list.`,
suggest: [
{
desc: `Remove the dependency \`${context.getSource(
declaredDependencyNode,
)}\``,
desc: `Remove the dependency \`${getSource(declaredDependencyNode)}\``,
fix(fixer) {
return fixer.removeRange(declaredDependencyNode.range!);
return fixer.removeRange(declaredDependencyNode.range);
},
},
],
@ -696,13 +725,13 @@ const rule: Rule.RuleModule = {
reportProblem({
node: declaredDependencyNode,
message:
`React Hook ${context.getSource(reactiveHook)} has a ` +
`React Hook ${getSource(reactiveHook)} has a ` +
`complex expression in the dependency array. ` +
'Extract it to a separate variable so it can be statically checked.',
});
}
return;
continue;
} else {
throw error;
}
@ -731,7 +760,7 @@ const rule: Rule.RuleModule = {
if (!isDeclaredInComponent) {
externalDependencies.add(declaredDependency);
}
});
}
}
const {
@ -782,9 +811,7 @@ const rule: Rule.RuleModule = {
const message =
`The '${construction.name.name}' ${depType} ${causation} the dependencies of ` +
`${reactiveHookName} Hook (at line ${
declaredDependenciesNode.loc!.start.line
}) ` +
`${reactiveHookName} Hook (at line ${declaredDependenciesNode.loc!.start.line}) ` +
`change on every render. ${advice}`;
let suggest: Rule.SuggestionReportDescriptor[] | undefined;
@ -838,7 +865,7 @@ const rule: Rule.RuleModule = {
// in some extra deduplication. We can't do this
// for effects though because those have legit
// use cases for over-specifying deps.
if (!isEffect && missingDependencies.size) {
if (!isEffect && missingDependencies.size > 0) {
suggestedDeps = collectRecommendations({
dependencies,
declaredDependencies: [], // Pretend we don't know
@ -854,7 +881,7 @@ const rule: Rule.RuleModule = {
return true;
}
const declaredDepKeys = declaredDependencies.map(dep => dep.key);
const sortedDeclaredDepKeys = declaredDepKeys.slice().sort();
const sortedDeclaredDepKeys = [...declaredDepKeys].sort();
return declaredDepKeys.join(',') === sortedDeclaredDepKeys.join(',');
}
@ -895,11 +922,7 @@ const rule: Rule.RuleModule = {
' ' +
(deps.size > 1 ? 'dependencies' : 'dependency') +
': ' +
joinEnglish(
Array.from(deps)
.sort()
.map(name => "'" + formatDependency(name) + "'"),
) +
joinEnglish([...deps].sort().map(name => "'" + formatDependency(name) + "'")) +
`. Either ${fixVerb} ${
deps.size > 1 ? 'them' : 'it'
} or remove the dependency array.`
@ -909,20 +932,20 @@ const rule: Rule.RuleModule = {
let extraWarning = '';
if (unnecessaryDependencies.size > 0) {
let badRef: string | null = null;
Array.from(unnecessaryDependencies.keys()).forEach(key => {
for (const key of unnecessaryDependencies.keys()) {
if (badRef !== null) {
return;
continue;
}
if (key.endsWith('.current')) {
badRef = key;
}
});
}
if (badRef !== null) {
extraWarning =
` Mutable values like '${badRef}' aren't valid dependencies ` +
"because mutating them doesn't re-render the component.";
} else if (externalDependencies.size > 0) {
const dep = Array.from(externalDependencies)[0];
const dep = [...externalDependencies][0];
// Don't show this warning for things that likely just got moved *inside* the callback
// because in that case they're clearly not referring to globals.
if (!scope.set.has(dep)) {
@ -971,11 +994,11 @@ const rule: Rule.RuleModule = {
` However, 'props' will change when *any* prop changes, so the ` +
`preferred fix is to destructure the 'props' object outside of ` +
`the ${reactiveHookName} call and refer to those specific props ` +
`inside ${context.getSource(reactiveHook)}.`;
`inside ${getSource(reactiveHook)}.`;
}
}
if (!extraWarning && missingDependencies.size) {
if (!extraWarning && missingDependencies.size > 0) {
// See if the user is trying to avoid specifying a callable prop.
// This usually means they're unaware of useCallback.
let missingCallbackDep: string | null = null;
@ -1041,7 +1064,7 @@ const rule: Rule.RuleModule = {
let id: Identifier;
let maybeCall: Node | null;
for (let i = 0; i < references.length; i++) {
id = references[i].identifier as Identifier;
id = references[i].identifier;
maybeCall = id.parent!;
// Try to see if we have setState(someExpr(missingDep)).
while (maybeCall != null && maybeCall !== componentScope.block) {
@ -1125,7 +1148,7 @@ const rule: Rule.RuleModule = {
reportProblem({
node: declaredDependenciesNode,
message:
`React Hook ${context.getSource(reactiveHook)} has ` +
`React Hook ${getSource(reactiveHook)} has ` +
// To avoid a long message, show the next actionable item.
(getWarningMessage(missingDependencies, 'a', 'missing', 'include') ||
getWarningMessage(unnecessaryDependencies, 'an', 'unnecessary', 'exclude') ||
@ -1158,7 +1181,11 @@ const rule: Rule.RuleModule = {
const reactiveHook = node.callee as Identifier | MemberExpression;
const reactiveHookName = (getNodeWithoutReactNamespace(reactiveHook) as Identifier)
.name;
const declaredDependenciesNode = node.arguments[callbackIndex + 1];
const maybeNode = node.arguments[callbackIndex + 1];
const declaredDependenciesNode =
maybeNode && !(maybeNode.type === 'Identifier' && maybeNode.name === 'undefined')
? maybeNode
: undefined;
const isEffect = /Effect($|[^a-z])/g.test(reactiveHookName);
// Check whether a callback is supplied. If there is no callback supplied
@ -1203,7 +1230,16 @@ const rule: Rule.RuleModule = {
isEffect,
);
return; // Handled
case 'Identifier':
case 'TSAsExpression':
visitFunctionWithDependencies(
callback.expression,
declaredDependenciesNode,
reactiveHook,
reactiveHookName,
isEffect,
);
return; // Handled
case 'Identifier': {
if (!declaredDependenciesNode) {
// No deps, no problems.
return; // Handled
@ -1221,7 +1257,7 @@ const rule: Rule.RuleModule = {
return; // Handled
}
// We'll do our best effort to find it, complain otherwise.
const variable = context.getScope().set.get(callback.name);
const variable = getScope(callback).set.get(callback.name);
if (variable == null || variable.defs == null) {
// If it's not in scope, we don't care.
return; // Handled
@ -1271,6 +1307,7 @@ const rule: Rule.RuleModule = {
break; // Unhandled
}
break; // Unhandled
}
default:
// useEffect(generateEffectBody(), []);
reportProblem({
@ -1358,33 +1395,33 @@ function collectRecommendations({
function createDepTree(): DepTree {
return {
isUsed: false,
isSatisfiedRecursively: false,
isSubtreeUsed: false,
children: new Map<string, never>(),
isUsed: false, // True if used in code
isSatisfiedRecursively: false, // True if specified in deps
isSubtreeUsed: false, // True if something deeper is used by code
children: new Map(), // Nodes for properties
};
}
// Mark all required nodes first.
// Imagine exclamation marks next to each used deep property.
dependencies.forEach((_, key) => {
for (const key of dependencies.keys()) {
const node = getOrCreateNodeByPath(depTree, key);
node.isUsed = true;
markAllParentsByPath(depTree, key, parent => {
parent.isSubtreeUsed = true;
});
});
}
// Mark all satisfied nodes.
// Imagine checkmarks next to each declared dependency.
declaredDependencies.forEach(({ key }) => {
for (const { key } of declaredDependencies) {
const node = getOrCreateNodeByPath(depTree, key);
node.isSatisfiedRecursively = true;
});
stableDependencies.forEach(key => {
}
for (const key of stableDependencies) {
const node = getOrCreateNodeByPath(depTree, key);
node.isSatisfiedRecursively = true;
});
}
// Tree manipulation helpers.
function getOrCreateNodeByPath(rootNode: DepTree, path: string): DepTree {
@ -1460,15 +1497,15 @@ function collectRecommendations({
const suggestedDependencies: string[] = [];
const unnecessaryDependencies = new Set<string>();
const duplicateDependencies = new Set<string>();
declaredDependencies.forEach(({ key }) => {
for (const { key } of declaredDependencies) {
// Does this declared dep satisfy a real need?
if (satisfyingDependencies.has(key)) {
if (!suggestedDependencies.includes(key)) {
// Good one.
suggestedDependencies.push(key);
} else {
if (suggestedDependencies.includes(key)) {
// Duplicate.
duplicateDependencies.add(key);
} else {
// Good one.
suggestedDependencies.push(key);
}
} else {
if (isEffect && !key.endsWith('.current') && !externalDependencies.has(key)) {
@ -1476,7 +1513,7 @@ function collectRecommendations({
// Such as resetting scroll when ID changes.
// Consider them legit.
// The exception is ref.current which is always wrong.
if (suggestedDependencies.indexOf(key) === -1) {
if (!suggestedDependencies.includes(key)) {
suggestedDependencies.push(key);
}
} else {
@ -1484,12 +1521,12 @@ function collectRecommendations({
unnecessaryDependencies.add(key);
}
}
});
}
// Then add the missing ones at the end.
missingDependencies.forEach(key => {
for (const key of missingDependencies) {
suggestedDependencies.push(key);
});
}
return {
suggestedDependencies,
@ -1545,7 +1582,7 @@ function getConstructionExpressionType(node: Node) {
}
return null;
case 'TypeCastExpression':
return getConstructionExpressionType(node.expression);
case 'AsExpression':
case 'TSAsExpression':
return getConstructionExpressionType(node.expression);
}
@ -1623,12 +1660,13 @@ function scanForConstructions({
while (currentScope !== scope && currentScope != null) {
currentScope = currentScope.upper!;
}
if (currentScope !== scope) {
if (
currentScope !== scope &&
// This reference is outside the Hook callback.
// It can only be legit if it's the deps array.
if (!isAncestorNodeOf(declaredDependenciesNode, reference.identifier)) {
return true;
}
!isAncestorNodeOf(declaredDependenciesNode, reference.identifier)
) {
return true;
}
}
return false;
@ -1653,7 +1691,6 @@ function getDependency(node: Node): Node {
if (
(parent.type === 'MemberExpression' || parent.type === 'OptionalMemberExpression') &&
parent.object === node &&
parent.property.type === 'Identifier' &&
parent.property.name !== 'current' &&
!parent.computed &&
!(
@ -1796,7 +1833,7 @@ function getReactiveHookCallbackIndex(
try {
name = analyzePropertyChain(node, null);
} catch (error) {
if (/Unsupported node type/.test(error.message)) {
if (/Unsupported node type/.test((error as Error).message)) {
return 0;
} else {
throw error;
@ -1842,12 +1879,12 @@ function fastFindReferenceWithParent(start: Node, target: Node): Node | null {
value.parent = item;
queue.push(value);
} else if (Array.isArray(value)) {
value.forEach(val => {
for (const val of value) {
if (isNodeLike(val)) {
val.parent = item;
queue.push(val);
}
});
}
}
}
}
@ -1870,7 +1907,7 @@ function joinEnglish(arr: string[]): string {
return s;
}
function isNodeLike(val: any): boolean {
function isNodeLike(val: unknown): val is Node {
return (
typeof val === 'object' &&
val !== null &&