eslint-plugin-react icon indicating copy to clipboard operation
eslint-plugin-react copied to clipboard

Bug: `jsx-no-leaked-render` faulty autofix

Open zaicevas opened this issue 3 years ago • 6 comments

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 avatar Jul 07 '22 20:07 zaicevas

cc @Belco90

zaicevas avatar Jul 07 '22 20:07 zaicevas

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.

ljharb avatar Jul 07 '22 20:07 ljharb

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

I'll try to take a look soon.

Belco90 avatar Jul 07 '22 20:07 Belco90

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

Belco90 avatar Jul 14 '22 09:07 Belco90

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

hduprat avatar Aug 10 '22 14:08 hduprat

@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.

Belco90 avatar Aug 10 '22 15:08 Belco90

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 && (...)}
    </>
  )

Jackman3005 avatar Aug 14 '22 11:08 Jackman3005

This should be fixed by #3353.

ljharb avatar Aug 14 '22 15:08 ljharb