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 using operator #2826

Merged
merged 1 commit into from Oct 13, 2020

Conversation

Moong0122
Copy link
Contributor

Since there were cases where indentation was not properly applied after #2808,
modified lib/rules/jsx-indent-props, and added test case in tests/lib/rules/jsx-indent-props.

Comment on lines 126 to +127
const useOperator = /^([ ]|[\t])*[:]/.test(src) || /^([ ]|[\t])*[?]/.test(src);
const useBracket = /^([ ]|[\t])*[<]/.test(src);
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!

@ljharb ljharb merged commit 237547e into jsx-eslint:master Oct 13, 2020
@ljharb ljharb mentioned this pull request Oct 14, 2020
ljharb pushed a commit to Hypnosphi/eslint-plugin-react that referenced this pull request Oct 15, 2020
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.

None yet

2 participants