diff --git a/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js b/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js index 3d60a36824d2..d283c826fdb9 100644 --- a/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js +++ b/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js @@ -1000,6 +1000,16 @@ const allTests = { `, errors: [conditionalError('Namespace.useConditionalHook')], }, + { + code: normalizeIndent` + // Invalid because optional chaining makes the hook call conditional. + // This *must* be invalid. + function ComponentWithConditionalHook(props) { + props.namespace?.useConditionalHook(); + } + `, + errors: [conditionalError('props.namespace?.useConditionalHook')], + }, { code: normalizeIndent` // Invalid because it's dangerous and might not warn otherwise. diff --git a/packages/eslint-plugin-react-hooks/src/code-path-analysis/code-path-analyzer.js b/packages/eslint-plugin-react-hooks/src/code-path-analysis/code-path-analyzer.js index 8dd76e57129b..cf2f9d3c91be 100644 --- a/packages/eslint-plugin-react-hooks/src/code-path-analysis/code-path-analyzer.js +++ b/packages/eslint-plugin-react-hooks/src/code-path-analysis/code-path-analyzer.js @@ -256,6 +256,7 @@ function preprocess(analyzer, node) { switch (parent.type) { // The `arguments.length == 0` case is in `postprocess` function. case 'CallExpression': + case 'OptionalCallExpression': if ( parent.optional === true && parent.arguments.length >= 1 && @@ -265,6 +266,7 @@ function preprocess(analyzer, node) { } break; case 'MemberExpression': + case 'OptionalMemberExpression': if (parent.optional === true && parent.property === node) { state.makeOptionalRight(); } @@ -448,11 +450,13 @@ function processCodePathToEnter(analyzer, node) { state.pushChainContext(); break; case 'CallExpression': + case 'OptionalCallExpression': if (node.optional === true) { state.makeOptionalNode(); } break; case 'MemberExpression': + case 'OptionalMemberExpression': if (node.optional === true) { state.makeOptionalNode(); } @@ -606,8 +610,10 @@ function processCodePathToExit(analyzer, node) { break; case 'CallExpression': + case 'OptionalCallExpression': case 'ImportExpression': case 'MemberExpression': + case 'OptionalMemberExpression': case 'NewExpression': case 'YieldExpression': state.makeFirstThrowablePathInTryBlock(); @@ -681,6 +687,7 @@ function postprocess(analyzer, node) { // The `arguments.length >= 1` case is in `preprocess` function. case 'CallExpression': + case 'OptionalCallExpression': if (node.optional === true && node.arguments.length === 0) { CodePath.getState(analyzer.codePath).makeOptionalRight(); } diff --git a/packages/eslint-plugin-react-hooks/src/rules/RulesOfHooks.ts b/packages/eslint-plugin-react-hooks/src/rules/RulesOfHooks.ts index ca82c99e2f55..d334039876d1 100644 --- a/packages/eslint-plugin-react-hooks/src/rules/RulesOfHooks.ts +++ b/packages/eslint-plugin-react-hooks/src/rules/RulesOfHooks.ts @@ -42,9 +42,18 @@ function isHook(node: Node): boolean { !node.computed && isHook(node.property) ) { + if ('optional' in node && node.optional) { + return true; + } const obj = node.object; const isPascalCaseNameSpace = /^[A-Z].*/; return obj.type === 'Identifier' && isPascalCaseNameSpace.test(obj.name); + } else if ( + node.type === 'OptionalMemberExpression' && + !node.computed && + isHook(node.property) + ) { + return true; } else { return false; } @@ -223,6 +232,43 @@ const rule = { > = []; const codePathSegmentStack: Array = []; const useEffectEventFunctions = new WeakSet(); + const recordedHooks = new WeakSet(); + const directlyReportedConditionalHooks = new WeakSet(); + + function recordHook(callee: Node): void { + if (recordedHooks.has(callee)) { + return; + } + recordedHooks.add(callee); + // Add the hook node to a map keyed by the code path segment. We will + // do full code path analysis at the end of our code path. + const reactHooksMap = last(codePathReactHooksMapStack); + const codePathSegment = last(codePathSegmentStack); + let reactHooks = reactHooksMap.get(codePathSegment); + if (!reactHooks) { + reactHooks = []; + reactHooksMap.set(codePathSegment, reactHooks); + } + reactHooks.push(callee); + } + + function reportConditionalHook(hook: Node): void { + directlyReportedConditionalHooks.add(hook); + const message = + `React Hook "${getSourceCode().getText(hook)}" is called ` + + 'conditionally. React Hooks must be called in the exact same order ' + + 'in every component render.'; + context.report({node: hook, message}); + } + + function isOptionalHookCall(node: any): boolean { + return ( + node.type === 'OptionalCallExpression' || + node.optional === true || + node.callee?.type === 'OptionalMemberExpression' || + node.callee?.optional === true + ); + } // For a given scope, iterate through the references and add all useEffectEvent definitions. We can // do this in non-Program nodes because we can rely on the assumption that useEffectEvent functions @@ -671,6 +717,7 @@ const rule = { !cycled && pathsFromStartToEnd !== allPathsFromStartToEnd && !isUseIdentifier(hook) && // `use(...)` can be called conditionally. + !directlyReportedConditionalHooks.has(hook) && !isInsideDoWhileLoop(hook) // wrapping do/while loops are checked separately. ) { const message = @@ -755,16 +802,14 @@ const rule = { // only being strict about hook calls for now. CallExpression(node) { if (isHook(node.callee)) { - // Add the hook node to a map keyed by the code path segment. We will - // do full code path analysis at the end of our code path. - const reactHooksMap = last(codePathReactHooksMapStack); - const codePathSegment = last(codePathSegmentStack); - let reactHooks = reactHooksMap.get(codePathSegment); - if (!reactHooks) { - reactHooks = []; - reactHooksMap.set(codePathSegment, reactHooks); + recordHook(node.callee); + if ( + isOptionalHookCall(node) && + isInsideComponentOrHook(node) && + !isUseIdentifier(node.callee) + ) { + reportConditionalHook(node.callee); } - reactHooks.push(node.callee); } // useEffectEvent: useEffectEvent functions can be passed by reference within useEffect as well as in @@ -798,6 +843,18 @@ const rule = { } }, + OptionalCallExpression(node: any) { + if (isHook(node.callee)) { + recordHook(node.callee); + if ( + isInsideComponentOrHook(node) && + !isUseIdentifier(node.callee) + ) { + reportConditionalHook(node.callee); + } + } + }, + Identifier(node) { // This identifier resolves to a useEffectEvent function, but isn't being referenced in an // effect or another event function. It isn't being called either.