Skip to content

Commit 1f7a901

Browse files
scyron6stephen cyrongaearon
authored
Fix false positive lint error with large number of branches (#24287)
* 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 <[email protected]> Co-authored-by: dan <[email protected]>
1 parent f56dfe9 commit 1f7a901

File tree

4 files changed

+81
-9
lines changed

4 files changed

+81
-9
lines changed

Diff for: packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js

+69
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,75 @@ const tests = {
325325
useHook();
326326
}
327327
`,
328+
`
329+
// Valid because the neither the conditions before or after the hook affect the hook call
330+
// Failed prior to implementing BigInt because pathsFromStartToEnd and allPathsFromStartToEnd were too big and had rounding errors
331+
const useSomeHook = () => {};
332+
333+
const SomeName = () => {
334+
const filler = FILLER ?? FILLER ?? FILLER;
335+
const filler2 = FILLER ?? FILLER ?? FILLER;
336+
const filler3 = FILLER ?? FILLER ?? FILLER;
337+
const filler4 = FILLER ?? FILLER ?? FILLER;
338+
const filler5 = FILLER ?? FILLER ?? FILLER;
339+
const filler6 = FILLER ?? FILLER ?? FILLER;
340+
const filler7 = FILLER ?? FILLER ?? FILLER;
341+
const filler8 = FILLER ?? FILLER ?? FILLER;
342+
343+
useSomeHook();
344+
345+
if (anyConditionCanEvenBeFalse) {
346+
return null;
347+
}
348+
349+
return (
350+
<React.Fragment>
351+
{FILLER ? FILLER : FILLER}
352+
{FILLER ? FILLER : FILLER}
353+
{FILLER ? FILLER : FILLER}
354+
{FILLER ? FILLER : FILLER}
355+
{FILLER ? FILLER : FILLER}
356+
{FILLER ? FILLER : FILLER}
357+
{FILLER ? FILLER : FILLER}
358+
{FILLER ? FILLER : FILLER}
359+
{FILLER ? FILLER : FILLER}
360+
{FILLER ? FILLER : FILLER}
361+
{FILLER ? FILLER : FILLER}
362+
{FILLER ? FILLER : FILLER}
363+
{FILLER ? FILLER : FILLER}
364+
{FILLER ? FILLER : FILLER}
365+
{FILLER ? FILLER : FILLER}
366+
{FILLER ? FILLER : FILLER}
367+
{FILLER ? FILLER : FILLER}
368+
{FILLER ? FILLER : FILLER}
369+
{FILLER ? FILLER : FILLER}
370+
{FILLER ? FILLER : FILLER}
371+
{FILLER ? FILLER : FILLER}
372+
{FILLER ? FILLER : FILLER}
373+
{FILLER ? FILLER : FILLER}
374+
{FILLER ? FILLER : FILLER}
375+
{FILLER ? FILLER : FILLER}
376+
{FILLER ? FILLER : FILLER}
377+
{FILLER ? FILLER : FILLER}
378+
{FILLER ? FILLER : FILLER}
379+
{FILLER ? FILLER : FILLER}
380+
{FILLER ? FILLER : FILLER}
381+
{FILLER ? FILLER : FILLER}
382+
{FILLER ? FILLER : FILLER}
383+
{FILLER ? FILLER : FILLER}
384+
{FILLER ? FILLER : FILLER}
385+
{FILLER ? FILLER : FILLER}
386+
{FILLER ? FILLER : FILLER}
387+
{FILLER ? FILLER : FILLER}
388+
{FILLER ? FILLER : FILLER}
389+
{FILLER ? FILLER : FILLER}
390+
{FILLER ? FILLER : FILLER}
391+
{FILLER ? FILLER : FILLER}
392+
{FILLER ? FILLER : FILLER}
393+
</React.Fragment>
394+
);
395+
};
396+
`,
328397
`
329398
// Valid because the neither the condition nor the loop affect the hook call.
330399
function App(props) {

Diff for: packages/eslint-plugin-react-hooks/src/RulesOfHooks.js

+10-9
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
* LICENSE file in the root directory of this source tree.
66
*/
77

8+
/* global BigInt */
89
/* eslint-disable no-for-of-loops/no-for-of-loops */
910

1011
'use strict';
@@ -175,7 +176,7 @@ export default {
175176
cyclic.add(cyclicSegment);
176177
}
177178

178-
return 0;
179+
return BigInt('0');
179180
}
180181

181182
// add the current segment to pathList
@@ -187,19 +188,19 @@ export default {
187188
}
188189

189190
if (codePath.thrownSegments.includes(segment)) {
190-
paths = 0;
191+
paths = BigInt('0');
191192
} else if (segment.prevSegments.length === 0) {
192-
paths = 1;
193+
paths = BigInt('1');
193194
} else {
194-
paths = 0;
195+
paths = BigInt('0');
195196
for (const prevSegment of segment.prevSegments) {
196197
paths += countPathsFromStart(prevSegment, pathList);
197198
}
198199
}
199200

200201
// If our segment is reachable then there should be at least one path
201202
// to it from the start of our code path.
202-
if (segment.reachable && paths === 0) {
203+
if (segment.reachable && paths === BigInt('0')) {
203204
cache.delete(segment.id);
204205
} else {
205206
cache.set(segment.id, paths);
@@ -246,7 +247,7 @@ export default {
246247
cyclic.add(cyclicSegment);
247248
}
248249

249-
return 0;
250+
return BigInt('0');
250251
}
251252

252253
// add the current segment to pathList
@@ -258,11 +259,11 @@ export default {
258259
}
259260

260261
if (codePath.thrownSegments.includes(segment)) {
261-
paths = 0;
262+
paths = BigInt('0');
262263
} else if (segment.nextSegments.length === 0) {
263-
paths = 1;
264+
paths = BigInt('1');
264265
} else {
265-
paths = 0;
266+
paths = BigInt('0');
266267
for (const nextSegment of segment.nextSegments) {
267268
paths += countPathsToEnd(nextSegment, pathList);
268269
}

Diff for: scripts/rollup/validate/eslintrc.cjs.js

+1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ module.exports = {
77
},
88
globals: {
99
// ES 6
10+
BigInt: 'readonly',
1011
Map: 'readonly',
1112
Set: 'readonly',
1213
Proxy: 'readonly',

Diff for: scripts/rollup/validate/eslintrc.cjs2015.js

+1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ module.exports = {
77
},
88
globals: {
99
// ES 6
10+
BigInt: 'readonly',
1011
Map: 'readonly',
1112
Set: 'readonly',
1213
Proxy: 'readonly',

0 commit comments

Comments
 (0)