jsx-eslint / eslint-plugin-react

React-specific linting rules for ESLint
MIT License
8.99k stars 2.77k forks source link

add autofix to sort-default-props #2062

Open hornta opened 5 years ago

hornta commented 5 years ago

Add same sorting behaviour to sort-default-props as to sort-props

pawelnvk commented 5 years ago

I would love to help with that one. Be patient, it will be my first contribution.

VincentLanglet commented 5 years ago

With foo = { bar: baz }.

{ ...foo, bar } = { bar }

And

{ bar, ...for } = { baz }

A fixer can break the code

ljharb commented 5 years ago

@VincentLanglet an incorrect fixer can break any code; a correct fixer for this rule must not move anything across the boundary created by a spread prop.

VincentLanglet commented 5 years ago

@ljharb I agree that a correct fixer for this rule must not move anything across the boundary created by a spread prop.

But then this fixer won't fix something like

{ a, ...foo, bar }

And actually the expected code is

{ a, bar, ...foo }

But

{ a, bar, ...foo } !== { a, ...foo, bar }

That's why I asked for an option to use the sort-default-props without having eslint asking me or my team to break my code.

ljharb commented 5 years ago

Right - the proper thing there is for the autofixer to ignore that change, leaving behind an un-auto-fixable error that you’re forced to manually address.

Perhaps I’ve misunderstood tho; you want an option that does not force sorting across spread boundaries, whether autofixed or not?

alexzherdev commented 5 years ago

FWIW the sort-prop-types autofixer does sort within the spread boundaries: https://github.com/yannickcr/eslint-plugin-react/blob/4a72e6a0784f7cc7f16f4574ce7f14de55540a73/tests/lib/rules/sort-prop-types.js#L1007-L1037

ljharb commented 5 years ago

In that case, this option requested in #2178 would, when enabled, just make the autofixer go from "partial" to "full".

webOS101 commented 4 years ago

Adding a note here to be sure that, when implemented, it moves any associated comments (as per #1940 )