@samitouri / QOS-React-2 / commits / 507f0fb372

Revert "[ESLint] Forbid top-level use*() calls (#16455)" (#16522)

This reverts commit 96eb703bbff49b7d52ad7d41ea18074dc8e7857a.

Sunil Pai committed Aug 21, 2019 at 10:20 UTC 507f0fb372705c4c6ac5288db3651019b1d0927b
2 files changed +13 -119
packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js
+12 -114
@@ -19,17 +19,8 @@ ESLintTester.setDefaultConfig({
19 },
20 });
21
22 -// ***************************************************
23 -// For easier local testing, you can add to any case:
24 -// {
25 -// skip: true,
26 -// --or--
27 -// only: true,
28 -// ...
29 -// }
30 -// ***************************************************
31 -
32 -const tests = {
22 +const eslintTester = new ESLintTester();
23 +eslintTester.run('react-hooks', ReactHooksESLintRule, {
24 valid: [
25 `
26 // Valid because components can use hooks.
@@ -232,20 +223,21 @@ const tests = {
223 (class {i() { useState(); }});
224 `,
225 `
235 - // Valid because they're not matching use[A-Z].
236 - fooState();
226 + // Currently valid although we *could* consider these invalid.
227 + // It doesn't make a lot of difference because it would crash early.
228 use();
229 _use();
230 + useState();
231 _useState();
232 + use42();
233 + useHook();
234 use_hook();
235 + React.useState();
236 `,
237 `
243 - // This is grey area.
244 - // Currently it's valid (although React.useCallback would fail here).
245 - // We could also get stricter and disallow it, just like we did
246 - // with non-namespace use*() top-level calls.
247 - const History = require('history-2.1.2');
248 - const browserHistory = History.useBasename(History.createHistory)({
238 + // Regression test for the popular "history" library
239 + const {createHistory, useBasename} = require('history-2.1.2');
240 + const browserHistory = useBasename(createHistory)({
241 basename: '/',
242 });
243 `,
@@ -677,63 +669,8 @@ const tests = {
669 conditionalError('useState'),
670 ],
671 },
680 - {
681 - code: `
682 - // Invalid because it's dangerous and might not warn otherwise.
683 - // This *must* be invalid.
684 - function useHook({ bar }) {
685 - let foo1 = bar && useState();
686 - let foo2 = bar || useState();
687 - let foo3 = bar ?? useState();
688 - }
689 - `,
690 - errors: [
691 - conditionalError('useState'),
692 - conditionalError('useState'),
693 - // TODO: ideally this *should* warn, but ESLint
694 - // doesn't plan full support for ?? until it advances.
695 - // conditionalError('useState'),
696 - ],
697 - },
698 - {
699 - code: `
700 - // Invalid because it's dangerous.
701 - // Normally, this would crash, but not if you use inline requires.
702 - // This *must* be invalid.
703 - // It's expected to have some false positives, but arguably
704 - // they are confusing anyway due to the use*() convention
705 - // already being associated with Hooks.
706 - useState();
707 - if (foo) {
708 - const foo = React.useCallback(() => {});
709 - }
710 - useCustomHook();
711 - `,
712 - errors: [
713 - topLevelError('useState'),
714 - topLevelError('React.useCallback'),
715 - topLevelError('useCustomHook'),
716 - ],
717 - },
718 - {
719 - code: `
720 - // Technically this is a false positive.
721 - // We *could* make it valid (and it used to be).
722 - //
723 - // However, top-level Hook-like calls can be very dangerous
724 - // in environments with inline requires because they can mask
725 - // the runtime error by accident.
726 - // So we prefer to disallow it despite the false positive.
727 -
728 - const {createHistory, useBasename} = require('history-2.1.2');
729 - const browserHistory = useBasename(createHistory)({
730 - basename: '/',
731 - });
732 - `,
733 - errors: [topLevelError('useBasename')],
734 - },
672 ],
736 -};
673 +});
674
675 function conditionalError(hook, hasPreviousFinalizer = false) {
676 return {
@@ -771,42 +708,3 @@ function genericError(hook) {
708 'Hook function.',
709 };
710 }
774 -
775 -function topLevelError(hook) {
776 - return {
777 - message:
778 - `React Hook "${hook}" cannot be called at the top level. React Hooks ` +
779 - 'must be called in a React function component or a custom React ' +
780 - 'Hook function.',
781 - };
782 -}
783 -
784 -// For easier local testing
785 -if (!process.env.CI) {
786 - let only = [];
787 - let skipped = [];
788 - [...tests.valid, ...tests.invalid].forEach(t => {
789 - if (t.skip) {
790 - delete t.skip;
791 - skipped.push(t);
792 - }
793 - if (t.only) {
794 - delete t.only;
795 - only.push(t);
796 - }
797 - });
798 - const predicate = t => {
799 - if (only.length > 0) {
800 - return only.indexOf(t) !== -1;
801 - }
802 - if (skipped.length > 0) {
803 - return skipped.indexOf(t) === -1;
804 - }
805 - return true;
806 - };
807 - tests.valid = tests.valid.filter(predicate);
808 - tests.invalid = tests.invalid.filter(predicate);
809 -}
810 -
811 -const eslintTester = new ESLintTester();
812 -eslintTester.run('react-hooks', ReactHooksESLintRule, tests);
packages/eslint-plugin-react-hooks/src/RulesOfHooks.js
+1 -5
@@ -432,13 +432,9 @@ export default {
432 'React Hook function.';
433 context.report({node: hook, message});
434 } else if (codePathNode.type === 'Program') {
435 + // For now, ignore if it's in top level scope.
436 // We could warn here but there are false positives related
437 // configuring libraries like `history`.
437 - const message =
438 - `React Hook "${context.getSource(hook)}" cannot be called ` +
439 - 'at the top level. React Hooks must be called in a ' +
440 - 'React function component or a custom React Hook function.';
441 - context.report({node: hook, message});
438 } else {
439 // Assume in all other cases the user called a hook in some
440 // random function callback. This should usually be true for