Skip to content

Commit 4a83035

Browse files
committed
Extend Namespace to PascalCase
1 parent b58dec9 commit 4a83035

File tree

2 files changed

+65
-48
lines changed

2 files changed

+65
-48
lines changed

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

Lines changed: 63 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ const tests = {
106106
({useHook() { useState(); }});
107107
const {useHook3 = () => { useState(); }} = {};
108108
({useHook = () => { useState(); }} = {});
109+
Namespace.useHook = () => { useState(); };
109110
`,
110111
`
111112
// Valid because hooks can call hooks.
@@ -191,47 +192,13 @@ const tests = {
191192
}
192193
}
193194
`,
194-
`
195-
// Currently valid.
196-
// We *could* make this invalid if we want, but it creates false positives
197-
// (see the FooStore case).
198-
class C {
199-
m() {
200-
This.useHook();
201-
Super.useHook();
202-
}
203-
}
204-
`,
205-
`
206-
// Valid although we *could* consider these invalid.
207-
// But it doesn't bring much benefit since it's an immediate runtime error anyway.
208-
// So might as well allow it.
209-
Hook.use();
210-
Hook._use();
211-
Hook.useState();
212-
Hook._useState();
213-
Hook.use42();
214-
Hook.useHook();
215-
Hook.use_hook();
216-
`,
217195
`
218196
// Valid -- this is a regression test.
219197
jest.useFakeTimers();
220198
beforeEach(() => {
221199
jest.useRealTimers();
222200
})
223201
`,
224-
`
225-
// Valid because that's a false positive we've seen quite a bit.
226-
// This is a regression test.
227-
class Foo extends Component {
228-
render() {
229-
if (cond) {
230-
FooStore.useFeatureFlag();
231-
}
232-
}
233-
}
234-
`,
235202
`
236203
// Valid because they're not matching use[A-Z].
237204
fooState();
@@ -240,16 +207,6 @@ const tests = {
240207
_useState();
241208
use_hook();
242209
`,
243-
`
244-
// This is grey area.
245-
// Currently it's valid (although React.useCallback would fail here).
246-
// We could also get stricter and disallow it, just like we did
247-
// with non-namespace use*() top-level calls.
248-
const History = require('history-2.1.2');
249-
const browserHistory = History.useBasename(History.createHistory)({
250-
basename: '/',
251-
});
252-
`,
253210
`
254211
// Regression test for some internal code.
255212
// This shows how the "callback rule" is more relaxed,
@@ -352,7 +309,6 @@ const tests = {
352309
if (c) {} else {}
353310
if (c) {} else {}
354311
if (c) {} else {}
355-
356312
// 10 hooks
357313
useHook();
358314
useHook();
@@ -392,6 +348,68 @@ const tests = {
392348
`,
393349
errors: [conditionalError('useConditionalHook')],
394350
},
351+
{
352+
code: `
353+
Hook.use();
354+
Hook._use();
355+
Hook.useState();
356+
Hook._useState();
357+
Hook.use42();
358+
Hook.useHook();
359+
Hook.use_hook();
360+
`,
361+
errors: [
362+
topLevelError('Hook.useState'),
363+
topLevelError('Hook.use42'),
364+
topLevelError('Hook.useHook'),
365+
],
366+
},
367+
{
368+
code: `
369+
class C {
370+
m() {
371+
This.useHook();
372+
Super.useHook();
373+
}
374+
}
375+
`,
376+
errors: [classError('This.useHook'), classError('Super.useHook')],
377+
},
378+
{
379+
code: `
380+
class Foo extends Component {
381+
render() {
382+
if (cond) {
383+
FooStore.useFeatureFlag();
384+
}
385+
}
386+
}
387+
`,
388+
errors: [classError('FooStore.useFeatureFlag')],
389+
},
390+
{
391+
code: `
392+
// Invalid because it's dangerous and might not warn otherwise.
393+
// This *must* be invalid.
394+
function ComponentWithConditionalHook() {
395+
if (cond) {
396+
Namespace.useConditionalHook();
397+
}
398+
}
399+
`,
400+
errors: [conditionalError('Namespace.useConditionalHook')],
401+
},
402+
{
403+
code: `
404+
// Extending the isHook check to [A-Z].*\.use
405+
// to not let it be called on top level
406+
const History = require('history-2.1.2');
407+
const browserHistory = History.useBasename(History.createHistory)({
408+
basename: '/',
409+
});
410+
`,
411+
errors: [topLevelError('History.useBasename')],
412+
},
395413
{
396414
code: `
397415
// Invalid because it's dangerous and might not warn otherwise.

packages/eslint-plugin-react-hooks/src/RulesOfHooks.js

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,9 @@ function isHook(node) {
3131
!node.computed &&
3232
isHook(node.property)
3333
) {
34-
// Only consider React.useFoo() to be namespace hooks for now to avoid false positives.
35-
// We can expand this check later.
3634
const obj = node.object;
37-
return obj.type === 'Identifier' && obj.name === 'React';
35+
const isPascalCaseNameSpace = /^[A-Z].*/;
36+
return obj.type === 'Identifier' && isPascalCaseNameSpace.test(obj.name);
3837
} else {
3938
return false;
4039
}

0 commit comments

Comments
 (0)