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

react/jsx-indent-props false positive in ternary operator #2841

Closed
polyrainbow opened this issue Oct 22, 2020 · 7 comments
Closed

react/jsx-indent-props false positive in ternary operator #2841

polyrainbow opened this issue Oct 22, 2020 · 7 comments

Comments

@polyrainbow
Copy link

Since 7.21.4 I get a react/jsx-indent-props false positive when using props in a ternary operator. I thought issue I'm having is the same as #2828 and thus would be fixed in 7.21.5, but it looks like #2828 is fixed in 7.21.5 and this is not. 7.21.3 works fine. See below for eslint config.

Error

Bildschirmfoto 2020-10-22 um 15 19 27

Code

import { h } from "preact";

const Component = (props) => {
  return props.prop
    ? <input
      type="email"
    />
    : "";
};

export default Component;
eslint-Config
{
  "env": {
    "browser": true,
    "commonjs": true,
    "es6": true,
    "jest": true
  },
  "globals": {
    "Atomics": "readonly",
    "SharedArrayBuffer": "readonly"
  },
  "parserOptions": {
    "ecmaVersion": 2020,
    "sourceType": "module"
  },
  "extends": [
    "standard",
    "eslint:recommended",
    "standard-react"
  ],
  "plugins": [
    "react-hooks",
    "jsx-props-no-empty-lines"
  ],
  "settings": {
    "react": {
      "pragma": "h",
      "version": "16.3"
    }
  },
  "rules": {
    "indent": [
      "error",
      2
    ],
    "linebreak-style": [
      "error",
      "unix"
    ],
    "quotes": [
      "error",
      "double"
    ],
    "semi": [
      "error",
      "always"
    ],
    "no-unused-vars": [
      "error", {
        "vars": "all",
        "args": "after-used",
        "ignoreRestSiblings": false
      }
    ],
    "eqeqeq": ["error"],
    "no-redeclare": ["error"],
    "no-empty": ["error"],
    "vars-on-top": ["error"],
    "no-eq-null": ["error"],
    "no-extra-boolean-cast": ["error"],
    "no-label-var": ["error"],
    "no-undefined": ["error"],
    "no-use-before-define": ["error"],
    "no-mixed-requires": ["error"],
    "camelcase": ["error"],
    "no-mixed-spaces-and-tabs": ["error"],
    "no-multi-assign": ["error"],
    "no-const-assign": ["error"],
    "no-var": ["error"],
    "prefer-const": ["error"],
    "operator-linebreak": ["error", "before"],
    "no-console": ["error"],
    "react-hooks/rules-of-hooks": "error",
    "react-hooks/exhaustive-deps": "warn",
    "jsx-props-no-empty-lines/no-empty-lines": "error",
    "jsx-quotes": ["error", "prefer-double"],
    "react/jsx-pascal-case": "error",
    "react/prop-types": [0],
    "react/jsx-closing-bracket-location": [0],
    "react/jsx-closing-tag-location": [0],
    "react/jsx-handler-names": [0],
    "react/jsx-indent-props": [
      "error",
      2
    ]
  }
}
@ljharb
Copy link
Member

ljharb commented Oct 22, 2020

This looks like a correct error to me, the code should be:

const Component = (props) => {
  return props.prop
    ? <input
        type="email"
      />
    : "";
};

@polyrainbow
Copy link
Author

@ljharb Thanks for the tip. But I wonder why the indentation convention you are using is the correct one. Is this convention enforced by the jsx-indent-props rule?

I'm asking because if I adapt my styling the way you suggest, then this is against eslint's own indent rule and eslint remarks that the indentation is now too high:

Bildschirmfoto 2020-10-22 um 22 39 15

If this is not a bug but a fix, then it seems to me that two contradicting styling conventions for multi-line expressions inside a ternary operator are tried to be enforced, and I wonder how I can adjust the settings of both rules to follow one consistent approach.

@ljharb
Copy link
Member

ljharb commented Oct 22, 2020

hmm, interesting. Personally, I surround all multiline jsx in parens, like this:

const Component = (props) => {
  return props.prop
    ? (
      <input
        type="email"
      />
    ): "";
};

which i believe would pass indent just fine. If you're not willing to do that, then I'm not sure off the top of my head how to make all the rule configs coexist, but I'm open to suggestions.

@polyrainbow
Copy link
Author

Yes, this works for me and passes both rules. Thank you. I don't prefer any convention over the other so basically I'm happy to adapt my codebase.

But in the end I think it is a matter of taste. The rule configs did coexist before 7.21.4. The change resulting in the error I'm getting was proposed in #647 and introduced with #2808

My suggestion is to allow the user the decision if the indent level should be increased by a preceding ? or : operator in the line above. Maybe this could be achieved via an additional setting called "increaseByTernaryOperator" or something like that. It could have a default setting of true.

I'm not a versed eslint developer but I could try to create a PR if you like.

@ljharb
Copy link
Member

ljharb commented Oct 25, 2020

I think a PR that adds an additional option that controls, in a multiline ternary, whether the ? and : are counted as part of the indentation would be reasonable.

@polyrainbow
Copy link
Author

I've created a PR: #2846

@susiwen8
Copy link

susiwen8 commented Dec 3, 2020

Facing same issue.

@ljharb ljharb closed this as completed in 66593e5 Dec 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants