Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

prohibit && conditional rendering in the return statement of a component #1979

Closed
graingert opened this issue Sep 12, 2018 · 9 comments
Closed

Comments

@graingert
Copy link

graingert commented Sep 12, 2018

// avoid
const WrapTingy = ({ cond }) => (
  cond && <Tingy/>
);
 
// that would error if cond is `undefined`, use this version instead:
 
const WrapTingy = ({ cond }) => (
  <>{cond && <Tingy/>}</>
);
@ljharb
Copy link
Member

ljharb commented Sep 12, 2018

This seems like a very niche use case; if it’s trying to address the issue of conditionally rendering the wrong thing, it’s not doing enough, and if it’s only trying to address the return error, I’d say the prop-types rule would handle that - iow, this component is missing a required propType.

@graingert
Copy link
Author

still a problem when you want to have an optional value:

// avoid
const WrapTingy = ({ items }) => (
  items && items.length && <Tingy items={items}/>
);
WrapTingy.propTypes = {
  items: propTypes.array
}

@ljharb
Copy link
Member

ljharb commented Sep 12, 2018

React.Fragment is only available in 16.2+; fragment syntax requires babel 7 - this isn’t really a broadly available solution yet.

I still feel like this is a runtime warning, not a linter warning. If we did handle it in a rule, we’d have to figure out if a propType is required or not (and this isn’t statically possible on a custom propType), and only error when it’s optional or when the propType is missing.

@graingert
Copy link
Author

graingert commented Sep 12, 2018

Other options without fragment are:

const WrapTingy = ({ cond }) => (
  cond ? <Tingy/> : null
);
const WrapTingy = ({ cond }) => (
  (cond && <Tingy/>) || null
);
const WrapTingy = ({ cond }) => (
  !!cond && <Tingy/>
);

@graingert
Copy link
Author

graingert commented Sep 12, 2018

I still feel like this is a runtime warning

This is only a runtime error when cond is undefined in development people might execute that branch

@harsh-tamr
Copy link

harsh-tamr commented Sep 25, 2018

or !!cond && <Tingy/>

@steelbrain
Copy link

Opened a PR for this @ #2888

@ljharb
Copy link
Member

ljharb commented Dec 26, 2020

@steelbrain this issue doesn't have "help wanted" or "accepted" labels, which means the new rule isn't necessarily something we'll accept. I'll take a look at the PR, though.

ljharb pushed a commit to steelbrain/eslint-plugin-react that referenced this issue Dec 29, 2020
ljharb pushed a commit to steelbrain/eslint-plugin-react that referenced this issue Dec 29, 2020
ljharb pushed a commit to steelbrain/eslint-plugin-react that referenced this issue Dec 29, 2020
@ljharb
Copy link
Member

ljharb commented Feb 7, 2022

I'm happy to keep discussing on #2888, but I think this can be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants