Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Wip] fix(useComponentExportOnlyModules): Proper evaluation of the hook #4429

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use biome_console::markup;
use biome_deserialize_macros::Deserializable;
use biome_js_syntax::{
export_ext::{AnyJsExported, ExportedItem},
AnyJsBindingPattern, AnyJsCallArgument, AnyJsExpression, AnyJsModuleItem, AnyJsStatement,
AnyJsBindingPattern, AnyJsExpression, AnyJsModuleItem, AnyJsStatement, JsCallExpression,
JsModule,
};
use biome_rowan::{AstNode, TextRange};
Expand Down Expand Up @@ -61,7 +61,7 @@ declare_lint_rule! {
///
/// ```jsx
/// import { memo } from 'react';
/// const Component = () => <></>
/// export const Component = () => <></>
/// export default memo(Component);
/// ```
///
Expand Down Expand Up @@ -289,42 +289,48 @@ impl Rule for UseComponentExportOnlyModules {
// Function that returns a standard React component
const REACT_HOOKS: [&str; 2] = ["memo", "forwardRef"];

/// Check if the function is a React Hook
fn is_hooked_component(f: &JsCallExpression) -> Option<bool> {
let fn_name = match f.callee().ok()? {
AnyJsExpression::JsIdentifierExpression(fn_name) => fn_name.text(),
AnyJsExpression::JsStaticMemberExpression(member) => member.member().ok()?.text(),
_ => return None,
};
if !REACT_HOOKS.contains(&fn_name.as_str()) {
return None;
}
let args = f.arguments().ok()?;
let itr = args
.args()
.into_iter()
.filter_map(Result::ok)
.collect::<Vec<_>>();
if itr.len() != 1 {
return None;
}
match &itr[0].as_any_js_expression()? {
AnyJsExpression::JsArrowFunctionExpression(_) => Some(true),
AnyJsExpression::JsFunctionExpression(_) => Some(true),
AnyJsExpression::JsIdentifierExpression(arg) => {
Some(Case::identify(&arg.name().ok()?.text(), false) == Case::Pascal)
}
_ => None,
}
}

/// Check if the exported item is a React component
fn is_exported_react_component(any_exported_item: &ExportedItem) -> bool {
if let Some(AnyJsExported::AnyJsExpression(AnyJsExpression::JsCallExpression(f))) =
if let Some(exported_item_id) = any_exported_item.identifier.clone() {
Case::identify(&exported_item_id.text(), false) == Case::Pascal
&& match any_exported_item.exported.clone() {
Some(exported) => !matches!(exported, AnyJsExported::TsEnumDeclaration(_)),
None => true,
}
} else if let Some(AnyJsExported::AnyJsExpression(AnyJsExpression::JsCallExpression(f))) =
any_exported_item.exported.clone()
{
if let Ok(AnyJsExpression::JsIdentifierExpression(fn_name)) = f.callee() {
if !REACT_HOOKS.contains(&fn_name.text().as_str()) {
return false;
}
let Ok(args) = f.arguments() else {
return false;
};
let itr = args
.args()
.into_iter()
.filter_map(Result::ok)
.collect::<Vec<_>>();
if itr.len() != 1 {
return false;
}
let AnyJsCallArgument::AnyJsExpression(AnyJsExpression::JsIdentifierExpression(arg)) =
&itr[0]
else {
return false;
};
let Ok(arg_name) = arg.name() else {
return false;
};
return Case::identify(&arg_name.text(), false) == Case::Pascal;
}
is_hooked_component(&f).unwrap_or(false)
} else {
false
}
let Some(exported_item_id) = any_exported_item.identifier.clone() else {
return false;
};
Case::identify(&exported_item_id.text(), false) == Case::Pascal
&& match any_exported_item.exported.clone() {
Some(exported) => !matches!(exported, AnyJsExported::TsEnumDeclaration(_)),
None => true,
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
const Component = () => <></>
const func = () => {}
export default func(Component)
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
assertion_line: 86
expression: invalid_default_wrapped_component.jsx
---
# Input
```jsx
const Component = () => <></>
const func = () => {}
export default func(Component)

```

# Diagnostics
```
invalid_default_wrapped_component.jsx:1:7 lint/nursery/useComponentExportOnlyModules ━━━━━━━━━━━━━━━

! Components should be exported.

> 1 │ const Component = () => <></>
│ ^^^^^^^^^
2 │ const func = () => {}
3 │ export default func(Component)

i Fast Refresh only works when a file only exports components.

i Consider separating component exports into a new file.

i If it is not a component, it may not be following the variable naming conventions.


```

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export const func = () => {};
export default memo(() => <></>);
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
assertion_line: 86
expression: invalid_hooked_default_component_with_non_component.jsx
---
# Input
```jsx
export const func = () => {};
export default memo(() => <></>);

```

# Diagnostics
```
invalid_hooked_default_component_with_non_component.jsx:1:14 lint/nursery/useComponentExportOnlyModules ━━━━━━━━━━

! Exporting a non-component with components is not allowed.

> 1 │ export const func = () => {};
│ ^^^^
2 │ export default memo(() => <></>);
3 │

i Fast Refresh only works when a file only exports components.

i Consider separating non-component exports into a new file.

i If it is a component, it may not be following the variable naming conventions.


```
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
export const Hoge = () => {}
export const Component = () => {}
const func = () => {}
export default memo(func)
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
assertion_line: 86
expression: invalid_hooked_non_component.jsx
---
# Input
```jsx
export const Hoge = () => {}
export const Component = () => {}
const func = () => {}
export default memo(func)

Expand All @@ -16,7 +17,7 @@ invalid_hooked_non_component.jsx:3:16 lint/nursery/useComponentExportOnlyModules

! Exporting a non-component with components is not allowed.

1 │ export const Hoge = () => {}
1 │ export const Component = () => {}
2 │ const func = () => {}
> 3 │ export default memo(func)
│ ^^^^^^^^^^
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export const func = () => {};
export default React.memo(() => <></>);
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
assertion_line: 86
expression: invalid_sub_hooked_default_component_with_non_component.jsx
---
# Input
```jsx
export const func = () => {};
export default React.memo(() => <></>);

```

# Diagnostics
```
invalid_sub_hooked_default_component_with_non_component.jsx:1:14 lint/nursery/useComponentExportOnlyModules ━━━━━━━━━━

! Exporting a non-component with components is not allowed.

> 1 │ export const func = () => {};
│ ^^^^
2 │ export default React.memo(() => <></>);
3 │

i Fast Refresh only works when a file only exports components.

i Consider separating non-component exports into a new file.

i If it is a component, it may not be following the variable naming conventions.


```
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { memo } from 'react';

const Component = () => <></>
export const Component = () => <></>

export default memo(Component);
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
assertion_line: 84
expression: valid_hooked_component.jsx
assertion_line: 86
expression: valid_default_hooked_component.jsx
---
# Input
```jsx
import { memo } from 'react';

const Component = () => <></>
export const Component = () => <></>

export default memo(Component);
```
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
export const sampleConst = 100
export function hoge () {
export function foo () {
return 100
}

export const nonComponent = foo(sampleConst)

export default Foo.bar(()=>{})
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
assertion_line: 84
assertion_line: 86
expression: valid_non_components_only.jsx
---
# Input
```jsx
export const sampleConst = 100
export function hoge () {
export function foo () {
return 100
}

export const nonComponent = foo(sampleConst)

export default Foo.bar(()=>{})

```
Loading