From c25016ce402c5019c44824106fd3028977f14878 Mon Sep 17 00:00:00 2001 From: nissy-dev Date: Sat, 13 May 2023 01:41:12 +0900 Subject: [PATCH 1/2] fix: false positive for useHookAtTopLevel --- .../nursery/use_hook_at_top_level.rs | 2 + .../nursery/useHookAtTopLevel/invalid.ts | 9 ++++ .../nursery/useHookAtTopLevel/invalid.ts.snap | 48 +++++++++++++++++++ .../specs/nursery/useHookAtTopLevel/valid.ts | 9 ++++ .../nursery/useHookAtTopLevel/valid.ts.snap | 19 ++++++++ 5 files changed, 87 insertions(+) create mode 100644 crates/rome_js_analyze/tests/specs/nursery/useHookAtTopLevel/invalid.ts create mode 100644 crates/rome_js_analyze/tests/specs/nursery/useHookAtTopLevel/invalid.ts.snap create mode 100644 crates/rome_js_analyze/tests/specs/nursery/useHookAtTopLevel/valid.ts create mode 100644 crates/rome_js_analyze/tests/specs/nursery/useHookAtTopLevel/valid.ts.snap diff --git a/crates/rome_js_analyze/src/semantic_analyzers/nursery/use_hook_at_top_level.rs b/crates/rome_js_analyze/src/semantic_analyzers/nursery/use_hook_at_top_level.rs index 7640cf3628c..bc7677ef630 100644 --- a/crates/rome_js_analyze/src/semantic_analyzers/nursery/use_hook_at_top_level.rs +++ b/crates/rome_js_analyze/src/semantic_analyzers/nursery/use_hook_at_top_level.rs @@ -62,6 +62,8 @@ fn enclosing_function_if_call_is_at_top_level(call: &JsCallExpression) -> Option | JsSyntaxKind::JS_VARIABLE_DECLARATOR | JsSyntaxKind::JS_VARIABLE_DECLARATOR_LIST | JsSyntaxKind::JS_VARIABLE_DECLARATION + | JsSyntaxKind::TS_AS_EXPRESSION + | JsSyntaxKind::TS_SATISFIES_EXPRESSION ) }); diff --git a/crates/rome_js_analyze/tests/specs/nursery/useHookAtTopLevel/invalid.ts b/crates/rome_js_analyze/tests/specs/nursery/useHookAtTopLevel/invalid.ts new file mode 100644 index 00000000000..17cf6cdc96d --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/nursery/useHookAtTopLevel/invalid.ts @@ -0,0 +1,9 @@ +const Component1 = () => { + useEffect() as []; +}; + +const Component2 = () => { + if (a == 1) { + Component1(); + } +}; diff --git a/crates/rome_js_analyze/tests/specs/nursery/useHookAtTopLevel/invalid.ts.snap b/crates/rome_js_analyze/tests/specs/nursery/useHookAtTopLevel/invalid.ts.snap new file mode 100644 index 00000000000..760c8e1736b --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/nursery/useHookAtTopLevel/invalid.ts.snap @@ -0,0 +1,48 @@ +--- +source: crates/rome_js_analyze/tests/spec_tests.rs +expression: invalid.ts +--- +# Input +```js +const Component1 = () => { + useEffect() as []; +}; + +const Component2 = () => { + if (a == 1) { + Component1(); + } +}; + +``` + +# Diagnostics +``` +invalid.ts:2:3 lint/nursery/useHookAtTopLevel ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This hook is being called indirectly and conditionally, but all hooks must be called in the exact same order in every component render. + + 1 │ const Component1 = () => { + > 2 │ useEffect() as []; + │ ^^^^^^^^^ + 3 │ }; + 4 │ + + i This is the call path until the hook. + + 5 │ const Component2 = () => { + > 6 │ if (a == 1) { + │ + > 7 │ Component1(); + │ ^^^^^^^^^^^^ + 8 │ } + 9 │ }; + + i For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order. + + i See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level + + +``` + + diff --git a/crates/rome_js_analyze/tests/specs/nursery/useHookAtTopLevel/valid.ts b/crates/rome_js_analyze/tests/specs/nursery/useHookAtTopLevel/valid.ts new file mode 100644 index 00000000000..3249a911230 --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/nursery/useHookAtTopLevel/valid.ts @@ -0,0 +1,9 @@ +/* does not generate diagnostics */ + +const Component1 = () => { + useEffect() as []; +}; + +const Component2 = () => { + useEffect() satisfies []; +}; diff --git a/crates/rome_js_analyze/tests/specs/nursery/useHookAtTopLevel/valid.ts.snap b/crates/rome_js_analyze/tests/specs/nursery/useHookAtTopLevel/valid.ts.snap new file mode 100644 index 00000000000..9ea13707109 --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/nursery/useHookAtTopLevel/valid.ts.snap @@ -0,0 +1,19 @@ +--- +source: crates/rome_js_analyze/tests/spec_tests.rs +expression: valid.ts +--- +# Input +```js +/* does not generate diagnostics */ + +const Component1 = () => { + useEffect() as []; +}; + +const Component2 = () => { + useEffect() satisfies []; +}; + +``` + + From 62e478c46a91116c2af738995e3aac295f6d54c4 Mon Sep 17 00:00:00 2001 From: nissy-dev Date: Sat, 13 May 2023 01:43:15 +0900 Subject: [PATCH 2/2] docs: update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d1cd7250c7b..551d1de0cd4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -79,6 +79,7 @@ when there are breaking changes. - Fix false positive diagnostics that [`useCamelCase`](https://docs.rome.tools/lint/rules/usecamelcase/) caused to default exported components - Fix false positive diagnostics that [`useCamelCase`](https://docs.rome.tools/lint/rules/usecamelcase/) caused to private class members - Fix false positive diagnostics that [`useHookAtTopLevel`](https://docs.rome.tools/lint/rules/usehookattoplevel/) caused to arrow functions, export default functions and function expressions. +- Fix false positive diagnostics that [`useHookAtTopLevel`](https://docs.rome.tools/lint/rules/usehookattoplevel/) caused to `as` or `satisfies` expression. - Fix false positive diagnostics that [`noHeadeScope`](https://docs.rome.tools/lint/rules/noheaderscope/) caused to custom components - Fix false negative diagnostics that [`noNoninteractiveElementToInteractiveRole`](https://docs.rome.tools/lint/rules/nononinteractiveelementtointeractiverole/) and [`noNoninteractiveTabindex`](https://docs.rome.tools/lint/rules/nononinteractivetabindex/) caused to non-interactive elements.