Skip to content
Merged
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
22 changes: 22 additions & 0 deletions .changeset/red-buckets-sing.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
---
'@biomejs/biome': patch
---

Fixed [#8288](https://github.com/biomejs/biome/issues/8288): Fixed the issue with false positive errors

This new change will ignore attribute and only show diagnostics for JSX Expressions

For example

Valid:

```jsx
<Something checked={isOpen && items.length} />
```

Invalid:
```jsx
const Component = () =>{
return isOpen && items.length
}
```
20 changes: 11 additions & 9 deletions crates/biome_js_analyze/src/lint/nursery/no_leaked_render.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ use biome_analyze::{
use biome_console::markup;
use biome_js_syntax::{
AnyJsExpression, JsConditionalExpression, JsLogicalExpression, JsLogicalOperator, JsSyntaxNode,
JsxExpressionAttributeValue, JsxExpressionChild, JsxTagExpression,
binding_ext::AnyJsBindingDeclaration,
JsxExpressionChild, JsxTagExpression, binding_ext::AnyJsBindingDeclaration,
jsx_ext::AnyJsxElement,
};
use biome_rowan::{AstNode, declare_node_union};
use biome_rule_options::no_leaked_render::NoLeakedRenderOptions;
Expand Down Expand Up @@ -106,7 +106,7 @@ impl Rule for NoLeakedRender {
let query = ctx.query();
let model = ctx.model();

if !is_inside_jsx_expression(query.syntax()) {
if !is_inside_jsx_expression(query.syntax()).unwrap_or_default() {
return None;
}

Expand Down Expand Up @@ -267,10 +267,12 @@ declare_node_union! {
pub NoLeakedRenderQuery = JsLogicalExpression | JsConditionalExpression
}

fn is_inside_jsx_expression(node: &JsSyntaxNode) -> bool {
node.ancestors().any(|ancestor| {
JsxExpressionChild::can_cast(ancestor.kind())
|| JsxExpressionAttributeValue::can_cast(ancestor.kind())
|| JsxTagExpression::can_cast(ancestor.kind())
})
fn is_inside_jsx_expression(node: &JsSyntaxNode) -> Option<bool> {
let parent = node.parent()?;

Some(
JsxExpressionChild::can_cast(parent.kind())
|| JsxTagExpression::can_cast(parent.kind())
|| AnyJsxElement::can_cast(parent.kind()),
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -56,28 +56,14 @@ const MyComponent3 = () => {
return <div>{maybeObject && (isFoo ? <Aaa /> : <Bbb />)}</div>;
};

const MyComponent4 = () => {
return <Something checked={v ? false : isChecked} />;
};

const MyComponent5 = () => {
return <Something checked={cond && isIndeterminate ? false : isChecked} />;
};

const isOpen1 = 0;
const Component7 = () => {
return <Popover open={isOpen1 && items.length > 0} />;
};

const Component8 = ({ count, title }) => {
const MyComponent4 = ({ count, title }) => {
return <div>{(((((count))))) && ((title))}</div>;
};

const Component9 = ({ data }) => {
const MyComponent5 = ({ data }) => {
return <div>{(((((data)))) && (((((data.value))))))}</div>;
};
}

const Component = ({ value }) => {
const MyComponent6 = ({ value }) => {
return <div>{(((value))) && <Item />}</div>;
};

Original file line number Diff line number Diff line change
Expand Up @@ -62,32 +62,18 @@ const MyComponent3 = () => {
return <div>{maybeObject && (isFoo ? <Aaa /> : <Bbb />)}</div>;
};

const MyComponent4 = () => {
return <Something checked={v ? false : isChecked} />;
};

const MyComponent5 = () => {
return <Something checked={cond && isIndeterminate ? false : isChecked} />;
};

const isOpen1 = 0;
const Component7 = () => {
return <Popover open={isOpen1 && items.length > 0} />;
};

const Component8 = ({ count, title }) => {
const MyComponent4 = ({ count, title }) => {
return <div>{(((((count))))) && ((title))}</div>;
};

const Component9 = ({ data }) => {
const MyComponent5 = ({ data }) => {
return <div>{(((((data)))) && (((((data.value))))))}</div>;
};
}

const Component = ({ value }) => {
const MyComponent6 = ({ value }) => {
return <div>{(((value))) && <Item />}</div>;
};


```

# Diagnostics
Expand Down Expand Up @@ -318,107 +304,16 @@ invalid.jsx:56:15 lint/nursery/noLeakedRender ━━━━━━━━━━━
```

```
invalid.jsx:60:29 lint/nursery/noLeakedRender ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
invalid.jsx:60:15 lint/nursery/noLeakedRender ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

i Potential leaked value that might cause unintended rendering.

59 │ const MyComponent4 = () => {
> 60 │ return <Something checked={v ? false : isChecked} />;
^^^^^^^^^^^^^^^^^^^^^
59 │ const MyComponent4 = ({ count, title }) => {
> 60 │ return <div>{(((((count))))) && ((title))}</div>;
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
61 │ };
62 │

i This happens when you use ternary operators in JSX with alternate values that could be variables.

i Replace with a safe alternate value like an empty string , null or another JSX element.


```

```
invalid.jsx:64:29 lint/nursery/noLeakedRender ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

i Potential leaked value that might cause unintended rendering.

63 │ const MyComponent5 = () => {
> 64 │ return <Something checked={cond && isIndeterminate ? false : isChecked} />;
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
65 │ };
66 │

i This happens when you use ternary operators in JSX with alternate values that could be variables.

i Replace with a safe alternate value like an empty string , null or another JSX element.


```

```
invalid.jsx:64:29 lint/nursery/noLeakedRender ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

i Potential leaked value that might cause unintended rendering.

63 │ const MyComponent5 = () => {
> 64 │ return <Something checked={cond && isIndeterminate ? false : isChecked} />;
│ ^^^^^^^^^^^^^^^^^^^^^^^
65 │ };
66 │

i JavaScript's && operator returns the left value when it's falsy (e.g., 0, NaN, ''). React will render that value, causing unexpected UI output.

i Make sure the condition is explicitly boolean.Use !!value, value > 0, or a ternary expression.


```

```
invalid.jsx:69:24 lint/nursery/noLeakedRender ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

i Potential leaked value that might cause unintended rendering.

67 │ const isOpen1 = 0;
68 │ const Component7 = () => {
> 69 │ return <Popover open={isOpen1 && items.length > 0} />;
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^
70 │ };
71 │

i JavaScript's && operator returns the left value when it's falsy (e.g., 0, NaN, ''). React will render that value, causing unexpected UI output.

i Make sure the condition is explicitly boolean.Use !!value, value > 0, or a ternary expression.


```

```
invalid.jsx:73:15 lint/nursery/noLeakedRender ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

i Potential leaked value that might cause unintended rendering.

72 │ const Component8 = ({ count, title }) => {
> 73 │ return <div>{(((((count))))) && ((title))}</div>;
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
74 │ };
75 │

i JavaScript's && operator returns the left value when it's falsy (e.g., 0, NaN, ''). React will render that value, causing unexpected UI output.

i Make sure the condition is explicitly boolean.Use !!value, value > 0, or a ternary expression.


```

```
invalid.jsx:77:16 lint/nursery/noLeakedRender ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

i Potential leaked value that might cause unintended rendering.

76 │ const Component9 = ({ data }) => {
> 77 │ return <div>{(((((data)))) && (((((data.value))))))}</div>;
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
78 │ };
79 │

i JavaScript's && operator returns the left value when it's falsy (e.g., 0, NaN, ''). React will render that value, causing unexpected UI output.

i Make sure the condition is explicitly boolean.Use !!value, value > 0, or a ternary expression.
Expand All @@ -427,15 +322,15 @@ invalid.jsx:77:16 lint/nursery/noLeakedRender ━━━━━━━━━━━
```

```
invalid.jsx:81:15 lint/nursery/noLeakedRender ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
invalid.jsx:68:15 lint/nursery/noLeakedRender ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

i Potential leaked value that might cause unintended rendering.

80 │ const Component = ({ value }) => {
> 81 │ return <div>{(((value))) && <Item />}</div>;
67 │ const MyComponent6 = ({ value }) => {
> 68 │ return <div>{(((value))) && <Item />}</div>;
│ ^^^^^^^^^^^^^^^^^^^^^^^
82 │ };
83
69 │ };
70

i JavaScript's && operator returns the left value when it's falsy (e.g., 0, NaN, ''). React will render that value, causing unexpected UI output.

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
/* should generate diagnostics */
const Component1 = () => {
return (
<Nested>
<SecondNested>{userId ? 1 : undefined} </SecondNested>
</Nested>
);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
expression: issue8288.invalid.jsx
---
# Input
```jsx
/* should generate diagnostics */
const Component1 = () => {
return (
<Nested>
<SecondNested>{userId ? 1 : undefined} </SecondNested>
</Nested>
);
};

```

# Diagnostics
```
issue8288.invalid.jsx:5:19 lint/nursery/noLeakedRender ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

i Potential leaked value that might cause unintended rendering.

3 │ return (
4 │ <Nested>
> 5 │ <SecondNested>{userId ? 1 : undefined} </SecondNested>
│ ^^^^^^^^^^^^^^^^^^^^^^
6 │ </Nested>
7 │ );

i This happens when you use ternary operators in JSX with alternate values that could be variables.

i Replace with a safe alternate value like an empty string , null or another JSX element.


```
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/* should not generate diagnostics */
function getValue() {
return Math.random() > 0.5;
}

export default function MyComponent() {
return <button onClick={getValue() ? getValue : undefined} />;
}

const Component1 = () => {
return isPending ? loader : userResults;
};

const Component2 = () => {
return someRandomId && !someRandomBooleanValue ? <ContentHere /> : <NotFound />;
};

// Ignore Attribute
const Component3 = () => {
return (
<NoErrorWithAttribute
error={firstBool && secondBool}
coerceError={!!(firstBool && secondBool)}
orderId={isChecked ? orderId : undefined}
/>
);
};

const Component4 = () => {
return (
<Nested>
<SecondNested>{userId ? 1 : null} </SecondNested>
</Nested>
);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
expression: issue8288.valid.jsx
---
# Input
```jsx
/* should not generate diagnostics */
function getValue() {
return Math.random() > 0.5;
}

export default function MyComponent() {
return <button onClick={getValue() ? getValue : undefined} />;
}

const Component1 = () => {
return isPending ? loader : userResults;
};

const Component2 = () => {
return someRandomId && !someRandomBooleanValue ? <ContentHere /> : <NotFound />;
};

// Ignore Attribute
const Component3 = () => {
return (
<NoErrorWithAttribute
error={firstBool && secondBool}
coerceError={!!(firstBool && secondBool)}
orderId={isChecked ? orderId : undefined}
/>
);
};

const Component4 = () => {
return (
<Nested>
<SecondNested>{userId ? 1 : null} </SecondNested>
</Nested>
);
};

```