[Bug]: Should jsx-no-leaked-render cover unintended return values as well?

[tommilimmot]

Is there an existing issue for this?

• I have searched the existing issues and my issue is unique
• My issue appears in the command-line and not only in the text editor

Description Overview

Conditionally rendering a string value with extra braces are hard to spot and can cause a runtime error.

Should jsx-no-leaked-render cover also return values?

type Props = {
  isInteractive: boolean
  label: string
}
const Test: React.FunctionComponent<Props> = ({ isInteractive, label }) => (
  <div>
    {/* Will render `{label}` (object) instead of a string when `isInteractive` is false */}
    { isInteractive
      ? <button>{label}</button>
      : {label}
    }
    
    { isInteractive && <button>{label}</button> }
    {/* ...same issue as above */}
    { !isInteractive && {label} }
  </div>
)

When isInteractive is false the code will return an object instead of a string, but it's easy to miss in the ternary case. The error is Uncaught Error: Objects are not valid as a React child (found: object with keys {label}). If you meant to render a collection of children, use an array instead.

Appears both in VS Code Eslint plugin (dbaeumer.vscode-eslint) and command line running npx eslint . with following config:

{
  ...
  "extends": [
    "eslint:recommended",
    "plugin:@typescript-eslint/recommended-type-checked",
    "plugin:react/recommended",
    "plugin:react-hooks/recommended"
  ],
  ...
  "parser": "@typescript-eslint/parser",
  "parserOptions": {
    "ecmaFeatures": {
      "jsx": true
    }
  },
  ...
  "plugins": ["react", "@typescript-eslint"],
  "rules": {
    "react/jsx-no-leaked-render": "error"
  },
  "settings": {
    "react": {
      "version": "detect"
    }
  }
}

Expected Behavior

Report extraneous braces around strings on conditional renders. Remove extra braces on fix.

const Test: React.FunctionComponent<Props> = ({ isInteractive, label }) => (
  <div>
    { isInteractive
      ? <button>{label}</button>
      : label
    }
    
    { isInteractive && <button>{label}</button> }
    { !isInteractive && label }
  </div>
)

eslint-plugin-react version

v7.33.2

eslint version

v8.47.0

node version

v20.2.0

[ljharb]

I'm confused, label is typed as a string. Is the TS type wrong?

[tommilimmot]

@ljharb I believe Typescript compiler can't guard against this issue because the React.FunctionComponent return type is ReactElement<any, any> | null and it can contain nested components, jsx elements and plaintext.

If we focus on the conditional, the issue is that one can accidentally return an object instead of evaluated string. So if the component is called with <Test label="Frank" isInteractive={false} />

{ !isInteractive && label } – ts return value evaluates to string, Frank
{ !isInteractive && {label} } – ts return value evaluates to object, {label: "Frank"}, and it will cause this error.

This case is still quite easy to spot, but it took a long time for me to understand the issue when using a ternary. It would be great if Eslint could spot this issue when Typescript can't.

[ljharb]

ah, i see what you mean.

I'm really unsure if 1) this behavior should be checked at all, or 2) checked in this rule, or 3) checked in its own rule. It's a very subtle case.

[tommilimmot]

Thank you for considering!

(I just realised my naming of the issue is a bit silly as both left and right side of the conditional can become the component return value. If I read correctly jsx-no-leaked-render rule covers only the left side currently.)