Rule proposal: detect unnecessary use of fragments

[moretti]

The idea behind fragments is to group a list of children without adding extra nodes to the DOM.
Fragments are completely redundant when wrapping a single child, so the rule should detect examples like:

<>
  <div />
</>

<Fragment>
  <div />
</Fragment>

and flag them as a warning/error.

Example of valid usage of fragments:

<>
  <div />
  <div />
</>

(any case where children.length > 1)

[ljharb]

This is a good rule (altho until enzyme supports rendering non-nodes, a fragment with a non-node child is still necessary, so perhaps that should be controllable via a separate option).

Both forms - syntax and API - should be checked.

[pawelnvk]

I would love to work on this one! Could you elaborate more about both options? How would look like "non-node" child? Code snippet would be helpful.

[golopot]

Other cases:

1. Looks useless

const Foo = () => (
  <div>
    <>
      <div />
      <div />
    </>
  </div>
)

2. Looks useless, but arguable.

const Goo = () => (
  <div>
    <>
      <div />
      <div />
    </>
    <section />
  </div>
)

3. Not useless if it is a child of a non-dom component.

const Hoo = () => (
  <Moo>
    <>
      <div />
      <div />
    </>
  </Moo>
)

[ljharb]

I do think it’s always useless to pass a fragment as a child to an html element; the rule should warn on that too.

[golopot]

Here comes the problem of whether a tag is a html element, also noted in #1752 (comment). Maybe in this case it is ok to maintain a list of html tags in this repo, since in #1752 an omission of an html tag name is false positive, whereas in this rule, an omission is false negative.

[ljharb]

It’s an html element if the tag name is lowercase and doesn’t have a dot in it; that’s how jsx works - that comment is about knowing whether it’s a valid tag name or not.

[ljharb]

I suppose the autofix for that case wouldn’t be as safe if it wasn’t a valid tag name; but we could still warn on it.

[TkDodo]

When can we expect this to be released? It's been some time since this has been finished ...

[paolostyle]

What if I have a component that looks like this (very trivial example, usually the stuff in the map is a bit more complicated):

const Component = ({ someArray }) => {
  return (
    <>
      {someArray.map(item => <span>{item}</span>)}
    </>
  );

The rule reports an error but I think there shouldn't be one. Obviously it is possible that there will be 0, 1 or more children inside fragment, but I think there should be way to disable the rule for these situations through config.

[ljharb]

@paolostyle in that case, you're already missing a key inside the array; however components in react 16 can render arrays directly, so there's no need for the fragment.

[paolostyle]

That was just an artificial example, I always have keys in arrays in actual code because I use a linting rule for that, I don't think that's relevant here.

My whole codebase is in Typescript and I can't return an array from function components (and 95% of them are function components), there's a couple of links to Typescript and React types GH issues in this SO thread: https://stackoverflow.com/questions/46643517/return-react-16-array-elements-in-typescript.

I suppose my example is actually just a workaround so I just won't use this rule until it's fixed in TS/React types. Looking at how old this issue is I probably won't ever use it but oh well.

[ljharb]

TypeScript having broken types for React is something that TS needs to fix; my suggested workaround would be to not use TS :-p (but that SO answer tells you how to patch the types so it works)