Skip to content

[bug] no-leaked-conditional-rendering and potential leaked empty string #853

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

Closed
mrudowski opened this issue Nov 13, 2024 · 2 comments · Fixed by #864
Closed

[bug] no-leaked-conditional-rendering and potential leaked empty string #853

mrudowski opened this issue Nov 13, 2024 · 2 comments · Fixed by #864
Labels
Status: Released The issue has been released Type: Bug Something isn't working

Comments

@mrudowski
Copy link

mrudowski commented Nov 13, 2024

Describe the bug

In docs we have example with empty string that breaks react native:

import React from "react";
 
function Example() {
  return <>{"" && <div />}</>;
  //        ^^
  //        - Possible unexpected value will be rendered (React Dom: renders nothing, React Native, with React below 18: crashes 💥).
}

So shouldn't no-leaked-conditional-rendering rule displays warning in this situations?

const someString = "";
{someString && <Something />}
const SampleComponent = ({someString}: {someString: string}) => {
  return (
    <div>
      {someString && <Something />}
    </div>
  )
}

Reproduction

No response

Expected behavior

I would expected warning

Potential leaked value someString that might cause unintentionally rendered values or rendering crashes.(@eslint-react/ no-leaked-conditional-rendering)

Platform and versions

Node 20.14.0
@eslint-react/eslint-plugin: 1.16.1

Stack trace

No response

Additional context

No response

@Rel1cx Rel1cx added the Type: Bug Something isn't working label Nov 13, 2024
@Rel1cx Rel1cx closed this as completed in 237b055 Nov 18, 2024
@Rel1cx Rel1cx added the Status: Released The issue has been released label Nov 20, 2024
@mrudowski
Copy link
Author

mrudowski commented Nov 21, 2024

Thanks for supporting "" but...
I think when we have a variable from props of type string, we cannot be sure if it is not "".
So we should warning user and propose !!anyStringVar solution - the same as we do with anyNumberVar

@Rel1cx Rel1cx reopened this Nov 21, 2024
Rel1cx added a commit that referenced this issue Nov 21, 2024
…onal-rendering' should also warn 'anyStringVar' when react version is lower than 18, closes #853
@Rel1cx
Copy link
Owner

Rel1cx commented Nov 21, 2024

Thanks for supporting "" but... I think when we have a variable from props of type string, we cannot be sure if it is not "". So we should warning user and propose !!anyStringVar solution - the same as we do with anyNumberVar

Thanks for pointing this out, the new PR will fix this, along with proper version detection!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Released The issue has been released Type: Bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants