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

[Fix] jsx-indent-props : Apply indentation when operator is used in front of the upper line #2808

Merged
merged 1 commit into from Oct 3, 2020

Conversation

Moong0122
Copy link
Contributor

@Moong0122 Moong0122 commented Sep 28, 2020

To apply indentation if there is an operator in front of the line above,
modified lib/rules/jsx-indent-props, and added test case in tests/lib/rules/jsx-indent-props.

PR fixes #647

@ljharb

This comment has been minimized.

@Moong0122

This comment has been minimized.

@xfournet
Copy link

xfournet commented Oct 12, 2020

There is a regression with this update, as show in this example. At line "key=3" and the next one the linter is now expecting an indent of 6 instead of 4. This is not the case for the next element in the array.
If the first element of the array is commented (the one with the ternary expression) the expected indentation is correctly expected to 4 again.

import React from 'react';

const x = false;

const Test = () => ([
  (x
    ? <div key="1" />
    : <div key="2" />),
  <div
    key="3"
    align="left"
  />,
  <div
    key="4"
    align="left"
  />,
]);

export default Test;


@Moong0122
Copy link
Contributor Author

@xfournet
First of all, I would like to say thank you for discovering the problem!
The cases I added didn't seem to solve the case you gave me.

@ljharb
I want to add a case, modify the code, and deliver the PR again. Can I create a new issue?

@ljharb
Copy link
Member

ljharb commented Oct 12, 2020

@Moong0122 sure - no need for an issue tho, if you just want to open a PR directly?

@Moong0122
Copy link
Contributor Author

@ljharb
In my opinion, some people may have similar errors, so I think it would be better to let them know that the problem is being solved through the issue :)

@xfournet
I'm trying to revise the code again by creating a new issue, can I use the example you told me?

@ljharb
Copy link
Member

ljharb commented Oct 13, 2020

@Moong0122 hopefully we could solve and release it quickly enough that that wouldn't be a problem :-)

@Moong0122
Copy link
Contributor Author

Moong0122 commented Oct 13, 2020

@ljharb
yes, then I'll send the PR directly as soon as possible!

@xfournet
Copy link

@Moong0122 sure you can freely use/adapt the example as you wish

@bukowskiadam
Copy link

We're having this problem now. Using 7.21.3 causes no issues, but the latest one causes us a lot of errors. 👀 Thanks for fixing it in advance!

@danrot
Copy link

danrot commented Oct 14, 2020

Just a side question I discovered because of this BC break: I realized that the indent rule that comes with eslint itself also formatted my JSX code as I had it in mind. Does it make sense to keep using that rule then? 🤔

@ljharb
Copy link
Member

ljharb commented Oct 14, 2020

@danrot yes; eslint's indent rule both doesn't format jsx in all the same ways our rule does. I suggest configuring indent to ignore JSX, as the airbnb config does.

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

Successfully merging this pull request may close these issues.

jsx-indent-props fails on lines following preceding operator
5 participants