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.

Sign up for GitHub

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

Jump to bottom

[Fix] jsx-indent-props : Apply indentation when using operator #2826

Merged
merged 1 commit into from Oct 13, 2020
Merged

[Fix] jsx-indent-props : Apply indentation when using operator #2826

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Expand Up @@ -5,6 +5,11 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel

## Unreleased

### Fixed
* [`jsx-indent-props`]: Apply indentation when using brackets ([#2826][] @Moong0122)

[#2826]: https://github.com/yannickcr/eslint-plugin-react/issues/2826

## [7.21.4] - 2020.10.09

### Fixed
Expand Down
4 changes: 4 additions & 0 deletions lib/rules/jsx-indent-props.js
View file
Open in desktop
Expand Up @@ -124,10 +124,14 @@ module.exports = {

const indent = regExp.exec(src);
const useOperator = /^([ ]|[\t])*[:]/.test(src) || /^([ ]|[\t])*[?]/.test(src);
const useBracket = /^([ ]|[\t])*[<]/.test(src);
Comment on lines 126 to +127
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it occurs to me that perhaps we could use eslint AST utilities rather than regexes to detect these things. have you explored that at all?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First, thanks for the quick feedback!
I haven't explored so far, can I ask where I can get more information about it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's https://github.com/jsx-eslint/jsx-ast-utils, and https://eslint.org/docs/developer-guide/working-with-rules that might help.

I'll land this as-is for now, and we can improve it in a followup.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ljharb
Checking if there are other test cases with indentation error,
I will follow up with reference to what you have informed me.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good!


line.currentOperator = false;
if (useOperator) {
line.isUsingOperator = true;
line.currentOperator = true;
} else if (useBracket) {
line.isUsingOperator = false;
}

return indent ? indent[0].length : 0;
Expand Down
17 changes: 17 additions & 0 deletions tests/lib/rules/jsx-indent-props.js
View file
Open in desktop
Expand Up @@ -38,6 +38,23 @@ ruleTester.run('jsx-indent-props', rule, {
'/>'
].join('\n'),
options: [2]
}, {
code: [
'const Test = () => ([',
' (x',
' ? <div key="1" />',
' : <div key="2" />),',
' <div',
' key="3"',
' align="left"',
' />,',
' <div',
' key="4"',
' align="left"',
' />,',
']);'
].join('\n'),
options: [2]
}, {
code: [
'<App',
Expand Down