Skip to content

Commit 0cb45f8

Browse files
committed
fix(linter): fix false positives in rules-of-hooks
1 parent 33e5a49 commit 0cb45f8

2 files changed

Lines changed: 70 additions & 41 deletions

File tree

crates/oxc_linter/src/rules/react/rules_of_hooks.rs

Lines changed: 70 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -362,39 +362,53 @@ fn is_somewhere_inside_component_or_hook(nodes: &AstNodes, node_id: NodeId) -> b
362362
)
363363
})
364364
.any(|(id, ident)| {
365-
ident.is_some_and(|name| {
366-
is_react_component_or_hook_name(&name) || is_memo_or_forward_ref_callback(nodes, id)
367-
})
365+
ident.is_some_and(|name| is_react_component_or_hook_name(&name))
366+
|| is_memo_or_forward_ref_callback(nodes, id)
368367
})
369368
}
370369

371370
fn get_declaration_identifier<'a>(
372371
nodes: &'a AstNodes<'a>,
373372
node_id: NodeId,
374373
) -> Option<Cow<'a, str>> {
375-
nodes.ancestor_ids(node_id).map(|id| nodes.kind(id)).find_map(|kind| {
376-
match kind {
377-
// const useHook = () => {};
378-
AstKind::VariableDeclaration(decl) if decl.declarations.len() == 1 => {
379-
decl.declarations[0].id.get_identifier().map(|id| Cow::Borrowed(id.as_str()))
380-
}
381-
// useHook = () => {};
382-
AstKind::AssignmentExpression(expr)
383-
if matches!(expr.operator, AssignmentOperator::Assign) =>
384-
{
385-
expr.left.get_identifier().map(std::convert::Into::into)
386-
}
387-
// const {useHook = () => {}} = {};
388-
// ({useHook = () => {}} = {});
389-
AstKind::AssignmentPattern(patt) => {
390-
patt.left.get_identifier().map(|id| Cow::Borrowed(id.as_str()))
374+
let node = nodes.get_node(node_id);
375+
376+
match node.kind() {
377+
AstKind::Function(Function { id: Some(id), .. }) => {
378+
// function useHook() {}
379+
// const whatever = function useHook() {};
380+
//
381+
// Function declaration or function expression names win over any
382+
// assignment statements or other renames.
383+
Some(Cow::Borrowed(id.name.as_str()))
384+
}
385+
AstKind::Function(_) | AstKind::ArrowFunctionExpression(_) => {
386+
let parent =
387+
nodes.ancestor_ids(node_id).skip(1).map(|node| nodes.get_node(node)).next()?;
388+
389+
match parent.kind() {
390+
AstKind::VariableDeclarator(decl) => {
391+
decl.id.get_identifier().map(|id| Cow::Borrowed(id.as_str()))
392+
}
393+
// useHook = () => {};
394+
AstKind::AssignmentExpression(expr)
395+
if matches!(expr.operator, AssignmentOperator::Assign) =>
396+
{
397+
expr.left.get_identifier().map(std::convert::Into::into)
398+
}
399+
// const {useHook = () => {}} = {};
400+
// ({useHook = () => {}} = {});
401+
AstKind::AssignmentPattern(patt) => {
402+
patt.left.get_identifier().map(|id| Cow::Borrowed(id.as_str()))
403+
}
404+
// { useHook: () => {} }
405+
// { useHook() {} }
406+
AstKind::ObjectProperty(prop) => prop.key.name(),
407+
_ => None,
391408
}
392-
// { useHook: () => {} }
393-
// { useHook() {} }
394-
AstKind::ObjectProperty(prop) => prop.key.name(),
395-
_ => None,
396409
}
397-
})
410+
_ => None,
411+
}
398412
}
399413

400414
/// # Panics
@@ -914,7 +928,22 @@ fn test() {
914928
915929
return <div>{state}</div>;
916930
}
917-
"
931+
// https://github.com/toeverything/AFFiNE/blob/0ec1995addbb09fb5d4af765d84cc914b2905150/packages/frontend/core/src/hooks/use-query.ts#L46
932+
",
933+
"const createUseQuery =
934+
(immutable: boolean): useQueryFn =>
935+
(options, config) => {
936+
const configWithSuspense: SWRConfiguration = useMemo(
937+
() => ({
938+
suspense: true,
939+
...config,
940+
}),
941+
[config],
942+
);
943+
944+
const useSWRFn = immutable ? useSWRImutable : useSWR;
945+
return useSWRFn(options ? () => ['cloud', options.query.id, options.variables] : null, options ? () => fetcher(options) : null, configWithSuspense);
946+
};"
918947
];
919948

920949
let fail = vec![
@@ -1500,13 +1529,22 @@ fn test() {
15001529
}
15011530
",
15021531
// errors: [functionError('use', 'notAComponent')],
1503-
"
1504-
export const notAComponent = () => {
1505-
return () => {
1506-
useState();
1507-
}
1508-
}
1509-
",
1532+
// React doesn't report on this https://github.com/facebook/react/blob/9daabc0bf97805be23f6131be4d84d063a3ff446/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js#L520-L530
1533+
// Even so, i think this is valid
1534+
// e.g:
1535+
// ```
1536+
// const useMyHook = notAComponent();
1537+
// function Foo () {
1538+
// useMyHook();
1539+
// }
1540+
// ```
1541+
// "
1542+
// export const notAComponent = () => {
1543+
// return () => {
1544+
// useState();
1545+
// }
1546+
// }
1547+
// ",
15101548
// errors: [functionError('use', 'notAComponent')],
15111549
"
15121550
const notAComponent = () => {

crates/oxc_linter/src/snapshots/react_rules_of_hooks.snap

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -650,15 +650,6 @@ source: crates/oxc_linter/src/tester.rs
650650
3use();
651651
╰────
652652
653-
eslint-plugin-react-hooks(rules-of-hooks): React Hook "useState" is called in function "Anonymous" that is neither a React function component nor a custom React Hook function. React component names must start with an uppercase letter. React Hook names must start with the word "use".
654-
╭─[rules_of_hooks.tsx:3:24]
655-
2 │ export const notAComponent = () => {
656-
3 │ ╭─▶ return () => {
657-
4 │ │ useState();
658-
5 │ ╰─▶ }
659-
6 │ }
660-
╰────
661-
662653
eslint-plugin-react-hooks(rules-of-hooks): React Hook "useState" is called in function "Anonymous" that is neither a React function component nor a custom React Hook function. React component names must start with an uppercase letter. React Hook names must start with the word "use".
663654
╭─[rules_of_hooks.tsx:2:35]
664655
1 │

0 commit comments

Comments
 (0)