[New] jsx-no-useless-fragment: add ignoreUnsafeChildren flag

[sbdchd]

With ignoreUnsafeChildren enabled, this rule will only warn about
useless fragments that removing would guarantee no change in behavior.

addresses some of: #2584

[sbdchd]

I think the ignoreNeedsMoreChildren name could use some work, open to ideas

[sbdchd]

maybe something along the lines of ignorePossiblyUnsafeExprs?

[codecov-io]

Codecov Report

Merging #2967 (5651663) into master (0d999ef) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #2967   +/-   ##
=======================================
  Coverage   97.58%   97.59%           
=======================================
  Files         110      110           
  Lines        7256     7266   +10     
  Branches     2643     2647    +4     
=======================================
+ Hits         7081     7091   +10     
  Misses        175      175           

Impacted Files	Coverage Δ
lib/rules/jsx-no-useless-fragment.js	98.88% <100.00%> (+0.13%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0d999ef...5651663. Read the comment docs.

[ljharb]

Per #2584 (comment), the intention was to add an option to only allow a prop value, and nothing else, to contain an otherwise useless fragment - because that's the only case where removing the fragment could in fact cause a runtime error.

[sbdchd]

@ljharb which fragments should we be warning about?

// function could be expecting a react node and raise an error at runtime if it got null instead
<></>
// children could be undefined
<>{children}</>
// foo could be undefined
<>{foo && <Foo/>}</>
// foo could be undefined
<>{foo?.map(x => <Bar x={x}/>)}</>

[ljharb]

In that case, you should be doing:

<></> // fair enough
// children could be undefined
{children ?? <></>}
// foo could be undefined
{foo ? <Foo/> : <></>}
// foo could be undefined
{foo?.map(x => <Bar x={x}/>) ?? <></>}

In general, if a component is so badly written that it crashes when it receives a nullish value instead of an element, i don't think it makes much sense for the consumer to need to worry about it. I also think that's an appropriate place for an eslint override comment, where you explain that the override is necessary because the other component is incompetently authored.

[fregante]

I think this would solve the only issue I have with the rule.

JSX.Element, React.Element and React.ReactNode are not interchangeable types, and however you pull them you end up with type errors.

If you have a React.FunctionComponent type for example you just cannot return a raw children, but you can return <>{children}</>

While <> is technically useless, TypeScript disagrees

Type '({ children, LoginPage, ignoreApiError, }: PropsWithChildren<RequireAuthProps>) => ReactNode' is not assignable to type 'FC<RequireAuthProps>'.
  Type 'ReactNode' is not assignable to type 'ReactElement<any, any>'.
    Type 'string' is not assignable to type 'ReactElement<any, any>'

[ljharb]

@fregante can you elaborate (maybe with a ts playground link) on what you're hoping this PR would fix?

[fregante]

I haven't looked at the code, but I assume ignoreUnsafeChildren means that <>{children}<> or <>{props.children}</> would be allowed, without allowing all expressions via allowExpressions

The setting might make more sense as allowExpressions: "children"

I think the entirety of #2584 (except #2584 (comment)) has this single complaint, so you have a lot more examples there.