From 15ebafaad7c1cd300446deff930d832f1f5c1d38 Mon Sep 17 00:00:00 2001 From: stephen cyron Date: Wed, 6 Apr 2022 18:37:36 -0400 Subject: [PATCH 1/5] Switched RulesOfHooks.js to use BigInt. Added test and updated .eslintrc.js to use es2020. --- .eslintrc.js | 3 + .../__tests__/ESLintRulesOfHooks-test.js | 69 +++++++++++++++++++ .../src/RulesOfHooks.js | 18 ++--- yarn.lock | 2 +- 4 files changed, 82 insertions(+), 10 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index 0bc90137a5c7..194dce6eae16 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -15,6 +15,9 @@ module.exports = { // Stop ESLint from looking for a configuration file in parent folders root: true, + env: { + es2020: 'true', + }, plugins: [ 'jest', diff --git a/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js b/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js index b6b0c8d4a967..044cc58f40c4 100644 --- a/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js +++ b/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js @@ -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 ( + + {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} + + ); + }; + `, ` // Valid because the neither the condition nor the loop affect the hook call. function App(props) { diff --git a/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js b/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js index 1273bf1d0c71..313ebbe68350 100644 --- a/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js +++ b/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js @@ -175,7 +175,7 @@ export default { cyclic.add(cyclicSegment); } - return 0; + return BigInt('0'); } // add the current segment to pathList @@ -187,11 +187,11 @@ 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); } @@ -199,7 +199,7 @@ export default { // 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); @@ -246,7 +246,7 @@ export default { cyclic.add(cyclicSegment); } - return 0; + return BigInt('0'); } // add the current segment to pathList @@ -258,11 +258,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); } diff --git a/yarn.lock b/yarn.lock index f376e74f8c5a..3d067e5be788 100644 --- a/yarn.lock +++ b/yarn.lock @@ -13629,7 +13629,7 @@ rc@^1.0.1, rc@^1.1.6, rc@^1.2.8: prop-types "^15.6.2" scheduler "^0.13.0" -react-is@^16.8.1, "react-is@npm:react-is": +"react-is@^16.12.0 || ^17.0.0", react-is@^16.8.1, react-is@^17.0.1, "react-is@npm:react-is": version "17.0.2" resolved "https://registry.yarnpkg.com/react-is/-/react-is-17.0.2.tgz#e691d4a8e9c789365655539ab372762b0efb54f0" integrity sha512-w2GsyukL62IJnlaff/nRegPQR94C/XXamvMWmSHRJ4y7Ts/4ocGRmTHvOs8PSE6pB3dWOrD/nueuU5sduBsQ4w== From 9db455ac91d8133550699573c348276fe9d33041 Mon Sep 17 00:00:00 2001 From: stephen cyron Date: Thu, 7 Apr 2022 17:40:12 -0400 Subject: [PATCH 2/5] Added BigInt as readonly global in eslintrc.cjs.js and eslintrc.cjs2015.js --- scripts/rollup/validate/eslintrc.cjs.js | 1 + scripts/rollup/validate/eslintrc.cjs2015.js | 1 + 2 files changed, 2 insertions(+) diff --git a/scripts/rollup/validate/eslintrc.cjs.js b/scripts/rollup/validate/eslintrc.cjs.js index 802141d6bc10..a76aa67155e2 100644 --- a/scripts/rollup/validate/eslintrc.cjs.js +++ b/scripts/rollup/validate/eslintrc.cjs.js @@ -7,6 +7,7 @@ module.exports = { }, globals: { // ES 6 + BigInt: 'readonly', Map: 'readonly', Set: 'readonly', Proxy: 'readonly', diff --git a/scripts/rollup/validate/eslintrc.cjs2015.js b/scripts/rollup/validate/eslintrc.cjs2015.js index b57956614577..32bb3e83aa3a 100644 --- a/scripts/rollup/validate/eslintrc.cjs2015.js +++ b/scripts/rollup/validate/eslintrc.cjs2015.js @@ -7,6 +7,7 @@ module.exports = { }, globals: { // ES 6 + BigInt: 'readonly', Map: 'readonly', Set: 'readonly', Proxy: 'readonly', From bee57d934269b31006332f5994977a23094b40ff Mon Sep 17 00:00:00 2001 From: stephen cyron Date: Thu, 7 Apr 2022 18:21:52 -0400 Subject: [PATCH 3/5] Added comment to RulesOfHooks.js that gets rid of BigInt eslint error --- packages/eslint-plugin-react-hooks/src/RulesOfHooks.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js b/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js index 313ebbe68350..06b75396d781 100644 --- a/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js +++ b/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js @@ -1,3 +1,4 @@ +/* global BigInt */ /** * Copyright (c) Facebook, Inc. and its affiliates. * From 89c4cf6e000601f0a02cb01d221ee82ac0ffa52b Mon Sep 17 00:00:00 2001 From: stephen cyron Date: Thu, 7 Apr 2022 18:33:14 -0400 Subject: [PATCH 4/5] Got rid of changes in .eslintrc.js and yarn.lock --- .eslintrc.js | 3 --- yarn.lock | 2 +- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index 194dce6eae16..0bc90137a5c7 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -15,9 +15,6 @@ module.exports = { // Stop ESLint from looking for a configuration file in parent folders root: true, - env: { - es2020: 'true', - }, plugins: [ 'jest', diff --git a/yarn.lock b/yarn.lock index 3d067e5be788..f376e74f8c5a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -13629,7 +13629,7 @@ rc@^1.0.1, rc@^1.1.6, rc@^1.2.8: prop-types "^15.6.2" scheduler "^0.13.0" -"react-is@^16.12.0 || ^17.0.0", react-is@^16.8.1, react-is@^17.0.1, "react-is@npm:react-is": +react-is@^16.8.1, "react-is@npm:react-is": version "17.0.2" resolved "https://registry.yarnpkg.com/react-is/-/react-is-17.0.2.tgz#e691d4a8e9c789365655539ab372762b0efb54f0" integrity sha512-w2GsyukL62IJnlaff/nRegPQR94C/XXamvMWmSHRJ4y7Ts/4ocGRmTHvOs8PSE6pB3dWOrD/nueuU5sduBsQ4w== From 8797f177b8b62928afc809b9c3879cd0b83886f9 Mon Sep 17 00:00:00 2001 From: dan Date: Thu, 7 Apr 2022 23:44:05 +0100 Subject: [PATCH 5/5] Move global down --- packages/eslint-plugin-react-hooks/src/RulesOfHooks.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js b/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js index 06b75396d781..31d54e813ae6 100644 --- a/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js +++ b/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js @@ -1,4 +1,3 @@ -/* global BigInt */ /** * Copyright (c) Facebook, Inc. and its affiliates. * @@ -6,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';