palantir / tslint-react

:orange_book: Lint rules related to React & JSX for TSLint.
Apache License 2.0
749 stars 76 forks source link

New rule suggestion: prefer-fragment #147

Closed pelotom closed 5 years ago

pelotom commented 6 years ago

Any time bare <div>s with no attributes are used, a fragment <>...</> should be used instead.

johnwiseheart commented 6 years ago

Hi @pelotom,

This sounds like a good idea for a rule. Unfortunately the TSLint team doesn't have the resources to develop it at the moment, but we would certainly be willing to accept PRs with this rule.

Thanks, TSLint Team

jkillian commented 6 years ago

Hmm, I'm a little concerned with this rule, I'm not sure it will be generally applicable. Having a bare div is semantically quite different than a fragment, for example a <div>:

I'm also curious which of the following elements the rule would affect in the hypothetical situation below:

public render() {
  return <header>
    <div>Option 1</div>
    <div><p>Option 2</p></div>
    <div />
  </header>;
}
banyan commented 6 years ago

@JKillian

That’s definitely fair enough. I'm wondering if we only care the root of div? Is this too arbitrary?

before:

public render() {
  return <div>
    <div>Option 1</div>
    <div><p>Option 2</p></div>
    <div />
  </div>;
}

after:

public render() {
  return <>
    <div>Option 1</div> // leave all divs as it is. 
    <div><p>Option 2</p></div>
    <div />
  </>;
}
aoberoi commented 6 years ago

I had a related idea: what if there was an option for jsx-wrap-multiline that enforced the preference of wrapping with a fragment, rather than parenthesis.

I currently have code like this:

const Foo = () => <>
  <h2>I'm a Foo</h2>
  <p>More content here</o>
</>;

I'm getting errors for the jsx-wrap-multiline rule, but I think in this case the fragment serves the purpose of the parenthesis. When there's only one element parent in the multiline jsx, I would still want the error.

I know this isn't exactly the rule being described here, but I think this is useful in determining what exactly the rules are trying to enforce.

adidahiya commented 5 years ago

This rule sounds like it could be useful but the idea needs to be fleshed out more and the edge cases covered. Discussion & work should continue in an eslint related repo due to the tslint deprecation timeline, see #210