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

Bug: jsx-no-leaked-render faulty autofix #3323

Closed
zaicevas opened this issue Jul 7, 2022 · 8 comments
Closed

Bug: jsx-no-leaked-render faulty autofix #3323

zaicevas opened this issue Jul 7, 2022 · 8 comments

Comments

@zaicevas
Copy link

zaicevas commented Jul 7, 2022

Version: 7.30.1
Config:

'react/jsx-no-leaked-render': ['error', { validStrategies: ['coerce', 'ternary'] }],

Before fix:

const MyComponent = () => {
  const items = []
  const breakpoint = { phones: true }

  return <div>{items.length > 0 && breakpoint.phones && <span />}</div>
}

Post fix:

const MyComponent = () => {
  const items = []
  const breakpoint = { phones: true }

  return <div>{!!!!!!!!!!!!!!!!!!!!items.length > 0 && breakpoint.phones && <span />}</div>
}

Expected:

const MyComponent = () => {
  const items = []
  const breakpoint = { phones: true }

  return <div>{items.length > 0 && !!breakpoint.phones && <span />}</div>
}

On top of that, the auto fix is quite slow.

@zaicevas
Copy link
Author

zaicevas commented Jul 7, 2022

cc @Belco90

@ljharb
Copy link
Member

ljharb commented Jul 7, 2022

The exclamation points definitely seem like a bug that should be fixed, and i agree that the > comparison shouldn't be touched since it can only ever be a boolean.

The autofixer's performance is something that can be improved separately, but isn't as concerning.

@Belco90
Copy link
Contributor

Belco90 commented Jul 7, 2022

I bet the bad performance is related to adding many negations.

I'll try to take a look soon.

@Belco90
Copy link
Contributor

Belco90 commented Jul 14, 2022

I identified the issue but I need more time to rework the fix functionality to solve it.

@hduprat
Copy link
Contributor

hduprat commented Aug 10, 2022

@Belco90 I created a PR to fix the multiple ! issue during autofix. Feel free to review it!

@Belco90
Copy link
Contributor

Belco90 commented Aug 10, 2022

@Belco90 I created a PR to fix the multiple ! issue during autofix. Feel free to review it!

That's amazing! I'll try to review it later today. Thanks.

hduprat pushed a commit to hduprat/eslint-plugin-react that referenced this issue Aug 12, 2022
It ensures jsx-eslint#3323 is fixed.
hduprat pushed a commit to hduprat/eslint-plugin-react that referenced this issue Aug 12, 2022
It ensures jsx-eslint#3323 is fixed.
@Jackman3005
Copy link

Jackman3005 commented Aug 14, 2022

I'm having a similar experience:

Original

{!sourceQuestData && loadingSourceQuest && (
  <StyledInfoLine>
    <Icon name="info" size={40} />
    <ActivityIndicator size="small" />
  </StyledInfoLine>
)}

Result

{!!!!!!!!!!!!!!!!!!!sourceQuestData && loadingSourceQuest && (
  <StyledInfoLine>
    <Icon name="info" size={40} />
    <ActivityIndicator size="small" />
  </StyledInfoLine>
)}

Similar to the OP, the !sourceQuestData should not have been touched. loadingSourceQuest is a boolean already, so it does not need to be touched either.

Adding to booleans

I also see it adding !! to variables that are already clearly a boolean (editMode is a boolean):

const showComponent = editMode || item.status === "COMPLETED";

  return (
    <>
      {!!showComponent && (...)}
    </>
  )

@ljharb
Copy link
Member

ljharb commented Aug 14, 2022

This should be fixed by #3353.

@ljharb ljharb closed this as completed Aug 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

5 participants