Skip to content

Commit

Permalink
Fix false positive lint error with large number of branches (#24287)
Browse files Browse the repository at this point in the history
* Switched RulesOfHooks.js to use BigInt. Added test and updated .eslintrc.js to use es2020.

* Added BigInt as readonly global in eslintrc.cjs.js and eslintrc.cjs2015.js

* Added comment to RulesOfHooks.js that gets rid of BigInt eslint error

* Got rid of changes in .eslintrc.js and yarn.lock

* Move global down

Co-authored-by: stephen cyron <stephen.cyron@fdmgroup.com>
Co-authored-by: dan <dan.abramov@gmail.com>
  • Loading branch information
3 people authored Apr 7, 2022
1 parent f56dfe9 commit 1f7a901
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,75 @@ const tests = {
useHook();
}
`,
`
// Valid because the neither the conditions before or after the hook affect the hook call
// Failed prior to implementing BigInt because pathsFromStartToEnd and allPathsFromStartToEnd were too big and had rounding errors
const useSomeHook = () => {};
const SomeName = () => {
const filler = FILLER ?? FILLER ?? FILLER;
const filler2 = FILLER ?? FILLER ?? FILLER;
const filler3 = FILLER ?? FILLER ?? FILLER;
const filler4 = FILLER ?? FILLER ?? FILLER;
const filler5 = FILLER ?? FILLER ?? FILLER;
const filler6 = FILLER ?? FILLER ?? FILLER;
const filler7 = FILLER ?? FILLER ?? FILLER;
const filler8 = FILLER ?? FILLER ?? FILLER;
useSomeHook();
if (anyConditionCanEvenBeFalse) {
return null;
}
return (
<React.Fragment>
{FILLER ? FILLER : FILLER}
{FILLER ? FILLER : FILLER}
{FILLER ? FILLER : FILLER}
{FILLER ? FILLER : FILLER}
{FILLER ? FILLER : FILLER}
{FILLER ? FILLER : FILLER}
{FILLER ? FILLER : FILLER}
{FILLER ? FILLER : FILLER}
{FILLER ? FILLER : FILLER}
{FILLER ? FILLER : FILLER}
{FILLER ? FILLER : FILLER}
{FILLER ? FILLER : FILLER}
{FILLER ? FILLER : FILLER}
{FILLER ? FILLER : FILLER}
{FILLER ? FILLER : FILLER}
{FILLER ? FILLER : FILLER}
{FILLER ? FILLER : FILLER}
{FILLER ? FILLER : FILLER}
{FILLER ? FILLER : FILLER}
{FILLER ? FILLER : FILLER}
{FILLER ? FILLER : FILLER}
{FILLER ? FILLER : FILLER}
{FILLER ? FILLER : FILLER}
{FILLER ? FILLER : FILLER}
{FILLER ? FILLER : FILLER}
{FILLER ? FILLER : FILLER}
{FILLER ? FILLER : FILLER}
{FILLER ? FILLER : FILLER}
{FILLER ? FILLER : FILLER}
{FILLER ? FILLER : FILLER}
{FILLER ? FILLER : FILLER}
{FILLER ? FILLER : FILLER}
{FILLER ? FILLER : FILLER}
{FILLER ? FILLER : FILLER}
{FILLER ? FILLER : FILLER}
{FILLER ? FILLER : FILLER}
{FILLER ? FILLER : FILLER}
{FILLER ? FILLER : FILLER}
{FILLER ? FILLER : FILLER}
{FILLER ? FILLER : FILLER}
{FILLER ? FILLER : FILLER}
{FILLER ? FILLER : FILLER}
</React.Fragment>
);
};
`,
`
// Valid because the neither the condition nor the loop affect the hook call.
function App(props) {
Expand Down
19 changes: 10 additions & 9 deletions packages/eslint-plugin-react-hooks/src/RulesOfHooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* LICENSE file in the root directory of this source tree.
*/

/* global BigInt */
/* eslint-disable no-for-of-loops/no-for-of-loops */

'use strict';
Expand Down Expand Up @@ -175,7 +176,7 @@ export default {
cyclic.add(cyclicSegment);
}

return 0;
return BigInt('0');
}

// add the current segment to pathList
Expand All @@ -187,19 +188,19 @@ export default {
}

if (codePath.thrownSegments.includes(segment)) {
paths = 0;
paths = BigInt('0');
} else if (segment.prevSegments.length === 0) {
paths = 1;
paths = BigInt('1');
} else {
paths = 0;
paths = BigInt('0');
for (const prevSegment of segment.prevSegments) {
paths += countPathsFromStart(prevSegment, pathList);
}
}

// If our segment is reachable then there should be at least one path
// to it from the start of our code path.
if (segment.reachable && paths === 0) {
if (segment.reachable && paths === BigInt('0')) {
cache.delete(segment.id);
} else {
cache.set(segment.id, paths);
Expand Down Expand Up @@ -246,7 +247,7 @@ export default {
cyclic.add(cyclicSegment);
}

return 0;
return BigInt('0');
}

// add the current segment to pathList
Expand All @@ -258,11 +259,11 @@ export default {
}

if (codePath.thrownSegments.includes(segment)) {
paths = 0;
paths = BigInt('0');
} else if (segment.nextSegments.length === 0) {
paths = 1;
paths = BigInt('1');
} else {
paths = 0;
paths = BigInt('0');
for (const nextSegment of segment.nextSegments) {
paths += countPathsToEnd(nextSegment, pathList);
}
Expand Down
1 change: 1 addition & 0 deletions scripts/rollup/validate/eslintrc.cjs.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ module.exports = {
},
globals: {
// ES 6
BigInt: 'readonly',
Map: 'readonly',
Set: 'readonly',
Proxy: 'readonly',
Expand Down
1 change: 1 addition & 0 deletions scripts/rollup/validate/eslintrc.cjs2015.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ module.exports = {
},
globals: {
// ES 6
BigInt: 'readonly',
Map: 'readonly',
Set: 'readonly',
Proxy: 'readonly',
Expand Down

0 comments on commit 1f7a901

Please sign in to comment.