Skip to content

Commit

Permalink
fix(noUselessFragments): fix result has invalid jsx when under JSX at…
Browse files Browse the repository at this point in the history
…tribute
  • Loading branch information
fireairforce committed Nov 27, 2024
1 parent cd1c8ec commit 77d99a9
Show file tree
Hide file tree
Showing 5 changed files with 95 additions and 9 deletions.
18 changes: 18 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,24 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b

- Fix [#4575](https://github.com/biomejs/biome/issues/4575), don't wrap selector identation after css comments. Contributed by @fireairforce

- Fix [#4553](https://github.com/biomejs/biome/issues/4553), `noUselessFragments` fix result has invalid syntax for JSX attribute, the follow code will fix:

```jsx
<Suspense fallback={<><span>Loading...</span></>}>
{children}
</Suspense>;
```

it will fix as:

```jsx
<Suspense fallback={<span>Loading...</span>}>
{children}
</Suspense>;
```



### JavaScript APIs

### Linter
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@ use biome_analyze::context::RuleContext;
use biome_analyze::{declare_lint_rule, FixKind, Rule, RuleDiagnostic, RuleSource};
use biome_console::markup;
use biome_js_factory::make::{
js_string_literal_expression, jsx_expression_child, jsx_string, jsx_string_literal,
jsx_tag_expression, token, JsxExpressionChildBuilder,
js_string_literal_expression, jsx_expression_attribute_value, jsx_expression_child, jsx_string,
jsx_string_literal, jsx_tag_expression, token, JsxExpressionChildBuilder,
};
use biome_js_syntax::{
AnyJsxChild, AnyJsxElementName, AnyJsxTag, JsLanguage, JsLogicalExpression,
AnyJsExpression, AnyJsxChild, AnyJsxElementName, AnyJsxTag, JsLanguage, JsLogicalExpression,
JsParenthesizedExpression, JsSyntaxKind, JsxChildList, JsxElement, JsxExpressionAttributeValue,
JsxExpressionChild, JsxFragment, JsxTagExpression, JsxText, T,
};
Expand Down Expand Up @@ -296,15 +296,14 @@ impl Rule for NoUselessFragments {
let node = ctx.query();
let mut mutation = ctx.root().begin();

let in_jsx_attr = node.syntax().grand_parent().map_or(false, |parent| {
let is_in_jsx_attr = node.syntax().grand_parent().map_or(false, |parent| {
JsxExpressionAttributeValue::can_cast(parent.kind())
});

let is_in_list = node
.syntax()
.parent()
.map_or(false, |parent| JsxChildList::can_cast(parent.kind()));

if is_in_list {
let new_child = match state {
NoUselessFragmentsState::Empty => None,
Expand All @@ -321,7 +320,6 @@ impl Rule for NoUselessFragments {
Some(grand_parent) => grand_parent.into_syntax(),
None => parent.into_syntax(),
};

let child = node
.children()
.iter()
Expand All @@ -337,7 +335,17 @@ impl Rule for NoUselessFragments {
if let Some(child) = child {
let new_node = match child {
AnyJsxChild::JsxElement(node) => {
Some(jsx_tag_expression(AnyJsxTag::JsxElement(node)).into_syntax())
let jsx_tag_expr = jsx_tag_expression(AnyJsxTag::JsxElement(node));
if is_in_jsx_attr {
let jsx_expr_attr_value = jsx_expression_attribute_value(
token(T!['{']),
AnyJsExpression::JsxTagExpression(jsx_tag_expr.clone()),
token(T!['}']),
);
Some(jsx_expr_attr_value.into_syntax())
} else {
Some(jsx_tag_expr.into_syntax())
}
}
AnyJsxChild::JsxFragment(node) => {
Some(jsx_tag_expression(AnyJsxTag::JsxFragment(node)).into_syntax())
Expand All @@ -358,7 +366,7 @@ impl Rule for NoUselessFragments {
}
}
AnyJsxChild::JsxExpressionChild(child) => {
if in_jsx_attr
if is_in_jsx_attr
|| !JsxTagExpression::can_cast(node.syntax().parent()?.kind())
{
child.expression().map(|expression| {
Expand Down
2 changes: 1 addition & 1 deletion crates/biome_js_analyze/tests/quick_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use std::{ffi::OsStr, fs::read_to_string, path::Path, slice};
#[ignore]
#[test]
fn quick_test() {
let input_file = Path::new("tests/specs/a11y/noAutofocus/invalid.jsx");
let input_file = Path::new("tests/specs/complexity/noUselessFragments/issue_4553.jsx");
let file_name = input_file.file_name().and_then(OsStr::to_str).unwrap();

let (group, rule) = parse_test_path(input_file);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<Suspense
fallback={
<>
<span>Loading...</span>
</>
}
>
{children}
</Suspense>;
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
expression: issue_4553.jsx
snapshot_kind: text
---
# Input
```jsx
<Suspense
fallback={
<>
<span>Loading...</span>
</>
}
>
{children}
</Suspense>;

```

# Diagnostics
```
issue_4553.jsx:3:9 lint/complexity/noUselessFragments FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Avoid using unnecessary Fragment.
1 │ <Suspense
2 │ fallback={
> 3 │ <>
│ ^^
> 4 │ <span>Loading...</span>
> 5 │ </>
│ ^^^
6 │ }
7 │ >
i A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
i Unsafe fix: Remove the Fragment
1 1 │ <Suspense
2 │ - ····fallback={
3 │ - ········<>
4 │ - ············<span>Loading...</span>
5 │ - ········</>
6 │ - ····}
2 │ + ····fallback={<span>Loading...</span>}
7 3 │ >
8 4 │ {children}
```

0 comments on commit 77d99a9

Please sign in to comment.