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

[Docs] `props-no-multi-spaces`: suggest using core rule instead #2463

Merged
merged 1 commit into from Oct 29, 2019

Conversation

@golopot
Copy link
Contributor

golopot commented Oct 13, 2019

No description provided.

@ljharb

This comment has been minimized.

Copy link
Collaborator

ljharb commented Oct 14, 2019

This is not accurate. The core rules do not provide sufficient jsx indentation control; the proper thing to do is disable the eslint core rule for all jsx, as the airbnb config does, and rely on this plugin to handle all jsx indentation.

@golopot

This comment has been minimized.

Copy link
Contributor Author

golopot commented Oct 14, 2019

I believe the capability of the core rule indent is in par with jsx-indent and jsx-indent-props, except for the option {indentLogicalExpressions: true}. Can you give provide some examples where the core rule is insufficient? Configuring indent to ignore jsx is very unergonomic.

@ljharb

This comment has been minimized.

Copy link
Collaborator

ljharb commented Oct 16, 2019

Not off the top of my head, but the airbnb config ignores jsx here: https://github.com/airbnb/javascript/blob/ab72ab9e90f403e90590f8815e0563248af10470/packages/eslint-config-airbnb-base/rules/style.js#L146 and sets all the react rules here: https://github.com/airbnb/javascript/blob/ab72ab9e90f403e90590f8815e0563248af10470/packages/eslint-config-airbnb/rules/react.js

If in fact every single permutation of our rules are possible with core, then this is fine to merge, but until then, the core indent rule should not be used with jsx whatsoever.

@golopot golopot force-pushed the golopot:docs-spaces branch from ea6b8c8 to 2da277f Oct 22, 2019
@golopot

This comment has been minimized.

Copy link
Contributor Author

golopot commented Oct 22, 2019

The core rule does not offer compatibility with every single permutation of our indent rules, so I will drop that part. I amended this pr to only deal with props-no-multi-spaces.

@golopot golopot changed the title [Docs]: indent rules are no longer needed with newer eslint [Docs] `props-no-multi-spaces`: suggest using core rule instead Oct 22, 2019
@ljharb ljharb force-pushed the golopot:docs-spaces branch from 2da277f to 39e4396 Oct 29, 2019
@ljharb
ljharb approved these changes Oct 29, 2019
@ljharb ljharb merged commit 39e4396 into yannickcr:master Oct 29, 2019
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.004%) to 97.524%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.