💅 noUselessFragments false negatives

[OliverJAsh]

Environment information

Rule name

noUselessFragments

Playground link

https://biomejs.dev/playground/?code=LwAvACAARQBTAEwAaQBuAHQAOgAgAGUAcgByAG8AcgAKAC8ALwAgAEIAaQBvAG0AZQA6ACAAbgBvACAAZQByAHIAbwByACAATCcKADwAcwBlAGMAdABpAG8AbgA%2BAAoAIAAgADwAPgAKACAAIAAgACAAPABkAGkAdgAgAC8APgAKACAAIAAgACAAPABkAGkAdgAgAC8APgAKACAAIAA8AC8APgAKADwALwBzAGUAYwB0AGkAbwBuAD4AOwAKAAoALwAvACAARQBTAEwAaQBuAHQAOgAgAGUAcgByAG8AcgAKAC8ALwAgAEIAaQBvAG0AZQA6ACAAbgBvACAAZQByAHIAbwByACAATCcKAHMAaABvAHcARgB1AGwAbABOAGEAbQBlACAAPwAgADwAPgB7AGYAdQBsAGwATgBhAG0AZQB9ADwALwA%2BACAAOgAgADwAPgB7AGYAaQByAHMAdABOAGEAbQBlAH0APAAvAD4AOwAKAAoALwAvACAARQBTAEwAaQBuAHQAOgAgAG4AbwAgAGUAcgByAG8AcgAgAEwnCgAvAC8AIABCAGkAbwBtAGUAOgAgAG4AbwAgAGUAcgByAG8AcgAgAEwnCgBjAG8AbgBzAHQAIABDADEAIAA9ACAAKAApACAAPQA%2BACAAPAA%2BAGYAbwBvADwALwA%2BADsACgAKAC8ALwAgAEUAUwBMAGkAbgB0ADoAIABuAG8AIABlAHIAcgBvAHIAIABMJwoALwAvACAAQgBpAG8AbQBlADoAIABuAG8AIABlAHIAcgBvAHIAIABMJwoAZABlAGMAbABhAHIAZQAgAGMAbwBuAHMAdAAgAGMAbwBuAGQAaQB0AGkAbwBuADoAIABiAG8AbwBsAGUAYQBuADsACgBjAG8AbgBzAHQAIABDADIAIAA9ACAAKAApACAAPQA%2BACAAPABkAGkAdgA%2BAHsAYwBvAG4AZABpAHQAaQBvAG4AIAA%2FACAAPAA%2BAGYAbwBvADwALwA%2BACAAOgAgADwAPgBiAGEAcgA8AC8APgB9ADwALwBkAGkAdgA%2BADsACgA%3D

Expected result

Copying the examples from react/jsx-no-useless-fragment:

// ESLint: error ✅
// Biome: no error ❌
<section>
  <>
    <div />
    <div />
  </>
</section>;

// ESLint: error ✅
// Biome: no error ❌
showFullName ? <>{fullName}</> : <>{firstName}</>;

And here are some other examples I found, where ESLint also doesn't error, but I think it maybe should?:

// ESLint: no error 🤔
// Biome: no error 🤔
const C1 = () => <>foo</>;

// ESLint: no error 🤔
// Biome: no error 🤔
declare const condition: boolean;
const C2 = () => <div>{condition ? <>foo</> : <>bar</>}</div>;

Code of Conduct

• I agree to follow Biome's Code of Conduct

[fireairforce]

The second case foo and bar is JsxText, if this case need to fix, it will be fix as:

declare const condition: boolean;
const C2 = () => <div>{condition ? "foo" :  "bar"}</div>;

maybe this is not a good way to fix the fragment here 🤔?

The first case same.

[OliverJAsh]

Why not? There might be a reason, especially if the equivalent ESLint rule isn't doing this, but I can't think what that reason might be. 🤔

[fireairforce]

I'm not sure... maybe you are right. I will fix the bug in your issue first. Let's try to ensure that the behavior is aligned with eslint. The other behaviour will need to be discussed(or refer to why eslint do't support this, IMO, eslint support this is easier than biome), i can also submit another pr to support this.

[ematipico]

// ESLint: no error 🤔
// Biome: no error 🤔
const C1 = () => <>foo</>;

In this example, it can't be an error, because you essentially change what's returned by the function. When you use a fragment, you signal that you're returning a JSX component. If you remove the Fragment, you'll return a variable. Removing the fragment will break your code (especially if your code is typed).

// ESLint: no error 🤔
// Biome: no error 🤔
declare const condition: boolean;
const C2 = () => <div>{condition ? <>foo</> : <>bar</>}</div>;

Here foo and bar aren't bindings, but text, which means having a space will break your code, when removing the fragment e.g <>foo example</> to foo example. In order to "fix" the code, we would need to transform foo to "foo", but this essentially changes the semantics of the code from a "JSX text" to a "JS string literal". This could also change the formatting heuristics. Hence, we shouldn't do anything, hence no violations.

[timtucker-dte]

The current fix for the rule breaks some TypeScript usecases.

Some examples of times when the autofix causes problems if an element is expected:

const foo ="Text";
const textValueAsElement = <>{foo}</>

const textAsElement = <>Text</>

function bar(): string {
    return "Text";
}
const textFunctionAsElement = <>{bar()}</>

[OliverJAsh]

Usually you can widen the type to ReactNode. Not sure if that would work in your example because I can't see where the variable is being used.

[timtucker-dte]

Widening the scope to ReactNode only works if you have control over the component that you're passing things into.

A more concrete example that causes problems (regardless of typing) is HTML entities.

The current rules seem to treat them as if they're just text strings, but they're not.

const blankValue = <>&nbsp;</>;

gets transformed into:

const blankValue = ' ';

So if you use it later on like:

<div>{blankValue}</div>

You wind up with the text string "& nbsp; " shown on the page instead of a non-breaking space.