Skip to content

Commit 507f0fb

Browse files
author
Sunil Pai
authored
Revert "[ESLint] Forbid top-level use*() calls (#16455)" (#16522)
This reverts commit 96eb703.
1 parent efa5dbe commit 507f0fb

File tree

2 files changed

+13
-119
lines changed

2 files changed

+13
-119
lines changed

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

Lines changed: 12 additions & 114 deletions
Original file line numberDiff line numberDiff line change
@@ -19,17 +19,8 @@ ESLintTester.setDefaultConfig({
1919
},
2020
});
2121

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, {
3324
valid: [
3425
`
3526
// Valid because components can use hooks.
@@ -232,20 +223,21 @@ const tests = {
232223
(class {i() { useState(); }});
233224
`,
234225
`
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.
237228
use();
238229
_use();
230+
useState();
239231
_useState();
232+
use42();
233+
useHook();
240234
use_hook();
235+
React.useState();
241236
`,
242237
`
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)({
249241
basename: '/',
250242
});
251243
`,
@@ -677,63 +669,8 @@ const tests = {
677669
conditionalError('useState'),
678670
],
679671
},
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-
},
735672
],
736-
};
673+
});
737674

738675
function conditionalError(hook, hasPreviousFinalizer = false) {
739676
return {
@@ -771,42 +708,3 @@ function genericError(hook) {
771708
'Hook function.',
772709
};
773710
}
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

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -432,13 +432,9 @@ export default {
432432
'React Hook function.';
433433
context.report({node: hook, message});
434434
} else if (codePathNode.type === 'Program') {
435+
// For now, ignore if it's in top level scope.
435436
// We could warn here but there are false positives related
436437
// 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});
442438
} else {
443439
// Assume in all other cases the user called a hook in some
444440
// random function callback. This should usually be true for

0 commit comments

Comments
 (0)