Skip to content

Commit

Permalink
Don't lint against Hooks after conditional throw
Browse files Browse the repository at this point in the history
Seems like this should be OK. Fixes #14038.

Now when tracking paths, we completely ignore segments that end in a throw. In https://eslint.org/docs/developer-guide/code-path-analysis I don't see a way to detect throws other than manually tracking them, so that's what I've done.
  • Loading branch information
sophiebits committed Oct 30, 2018
1 parent 169f935 commit b38a292
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,15 @@ eslintTester.run('react-hooks', ReactHooksESLintRule, {
useState();
}
`,
`
// Valid because exceptions abort rendering
function RegressionTest() {
if (page == null) {
throw new Error('oh no!');
}
useState();
}
`,
],
invalid: [
{
Expand Down
21 changes: 20 additions & 1 deletion packages/eslint-plugin-react-hooks/src/RulesOfHooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,20 +71,25 @@ export default {
create(context) {
const codePathReactHooksMapStack = [];
const codePathSegmentStack = [];
const codePathThrowingSegmentsStack = [];
return {
// Maintain code segment path stack as we traverse.
onCodePathSegmentStart: segment => codePathSegmentStack.push(segment),
onCodePathSegmentEnd: () => codePathSegmentStack.pop(),

// Maintain code path stack as we traverse.
onCodePathStart: () => codePathReactHooksMapStack.push(new Map()),
onCodePathStart: () => {
codePathReactHooksMapStack.push(new Map())
codePathThrowingSegmentsStack.push(new Set())
},

// Process our code path.
//
// Everything is ok if all React Hooks are both reachable from the initial
// segment and reachable from every final segment.
onCodePathEnd(codePath, codePathNode) {
const reactHooksMap = codePathReactHooksMapStack.pop();
const throwingSegments = codePathThrowingSegmentsStack.pop();
if (reactHooksMap.size === 0) {
return;
}
Expand Down Expand Up @@ -115,6 +120,10 @@ export default {
*/

function countPathsFromStart(segment) {
if (throwingSegments.has(segment)) {
return 0;
}

const {cache} = countPathsFromStart;
let paths = cache.get(segment.id);

Expand Down Expand Up @@ -175,6 +184,10 @@ export default {
*/

function countPathsToEnd(segment) {
if (throwingSegments.has(segment)) {
return 0;
}

const {cache} = countPathsToEnd;
let paths = cache.get(segment.id);

Expand Down Expand Up @@ -462,6 +475,12 @@ export default {
reactHooks.push(node.callee);
}
},

ThrowStatement(node) {
const throwingSegments = last(codePathThrowingSegmentsStack);
const codePathSegment = last(codePathSegmentStack);
throwingSegments.add(codePathSegment);
}
};
},
};
Expand Down

0 comments on commit b38a292

Please sign in to comment.