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

jsx-indent-props: Enforcing incorrect indents when using a component with props in ternary before #3146

Closed
tobiaswaltl opened this issue Nov 19, 2021 · 12 comments · Fixed by #3157

Comments

@tobiaswaltl
Copy link

I experience some odd behavior of jsx-indent-props depending on a ternary before the actual component.
When having a component with props within the ternary and not having the whole ternary in one line, the rule enforces more indents for the subsequent component than it should.

I would expect the ternary not having any impact on the returning component.

Tested versions: 7.27.0 and 7.27.1

import React from 'react';

// works as expected - no props for the component in ternary
export const LintExample = () => {
  const foo = true
    ? <div>test</div>
    : false;

  return <div
    id="id"
  >
    test
  </div>;
};

// works as expected - props for the component in ternary but ternary in one line
export const LintExample1 = () => {
  const foo = true ? <div id="id">test</div> /* added prop "id" here*/ : false;

  return <div
    id="id"
  >
    test
  </div>;
};

// does not work as expected - props for the component in ternary and ternary not in one line
export const LintExample2 = () => {
  const foo = true
    ? <div id="id">test</div> // added prop "id" here
    : false;

  return <div
    id="id" // linter complains: ESLint: Expected indentation of 6 space characters but found 4.(react/jsx-indent-props)
  >
    test
  </div>;
};
@ljharb
Copy link
Member

ljharb commented Nov 20, 2021

Can you provide your rule config?

@tobiaswaltl
Copy link
Author

My rule config is as follows (retrieved via eslint --print-config lint-example.tsx so there shouldn't be any overrides in the cascade that I might have missed) :

"react/jsx-indent-props": [
      "error",
      2
    ],

@ljharb
Copy link
Member

ljharb commented Nov 22, 2021

Interesting; I'm having trouble reproducing your issue - in other words, all three examples are failing the rule in my local tests.

@tobiaswaltl
Copy link
Author

Weird, I just double-checked the example and in my case only the 3rd one fails the rule.
Version of eslint-plugin-react is 7.27.1.
Version of eslint is 7.32.0. I also just tried it with the current version 8.3.0 and again only the 3rd example failed.

Do you agree that the expected behavior would be that none of the examples fails the rule for the given rule config?

@ljharb
Copy link
Member

ljharb commented Nov 24, 2021

I'm not sure what the expected behavior should be; I would expect multiline jsx to always be wrapped in parens, so it'd look like this:

export const LintExample2 = () => {
  const foo = true
    ? <div id="id">test</div> // added prop "id" here
    : false;

  return (
    <div
      id="id"
    >
      test
    </div>
  );
};

@tobiaswaltl
Copy link
Author

With parens, everything is fine. However, the LintExample2 without parens is also valid syntax. So in that case I would expect that the props have to be indented by one level while the rule asks me to indent it by two levels.

@ljharb
Copy link
Member

ljharb commented Dec 8, 2021

I see what you're saying. While I find that indentation style horribly confusing, it does seem like it should be accepted. A PR with failing test cases, and ideally a fix, would be appreciated.

@tobiaswaltl
Copy link
Author

Ok, great! I'll try to submit one by the end of next week.

tobiaswaltl pushed a commit to tobiaswaltl/eslint-plugin-react that referenced this issue Dec 14, 2021
tobiaswaltl pushed a commit to tobiaswaltl/eslint-plugin-react that referenced this issue Dec 14, 2021
tobiaswaltl pushed a commit to tobiaswaltl/eslint-plugin-react that referenced this issue Dec 15, 2021
tobiaswaltl pushed a commit to tobiaswaltl/eslint-plugin-react that referenced this issue Dec 21, 2021
@Kegulf
Copy link

Kegulf commented Feb 25, 2022

This now breaks several indent uses without any JSX in JS-files in my project

Also breaks this type of formated ternaries with JSX:

@Kegulf
Copy link

Kegulf commented Feb 25, 2022

@ljharb @tobiaswaltl

@Kegulf
Copy link

Kegulf commented Feb 25, 2022

Still broken in 7.29.0

Sorry if that first message felt harsh btw 😅

@ljharb
Copy link
Member

ljharb commented Feb 25, 2022

@Kegulf you say "still", but this change was first released in v7.2.9.0, about 12 hours ago. A fix is coming shortly; see #3218.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants