search

leo-buneev / eslint-plugin-sort-keys-fix

Fork of https://eslint.org/docs/rules/sort-keys that allows automatic fixing
95 stars 22 forks source link

Do not move properties across spread #3

Closed gajus closed 4 years ago

gajus commented 5 years ago

See:

https://github.com/eslint/eslint/issues/10715#issuecomment-409745176

hcharley commented 4 years ago

This issue being open scares me away from using this project. This could have serious implications for anyone using this. Right?

gajus commented 4 years ago

This issue being open scares me away from using this project. This could have serious implications for anyone using this. Right?

Without this fix, the project is unusable in a codebase that uses spread.

TSMMark commented 4 years ago

This is unfortunate. Anyone have a branch where this is fixed and can submit a PR? Otherwise, any pointers on how to implement a fix? I'm no expert but I've written one autofixing eslint rule before, maybe I could take a stab at it.

leo-buneev commented 4 years ago

Hi folks,

I've personally used this project not on entire codebase, but on particular files, so didn't implement spread case properly.

Seeing as this is fairly popular request, will add handling for spread in new version this week.

But be aware that changing order of keys in some very rare cases may affect how your code works (I believe that's why autofix its not implemented in core eslint).

piranna commented 4 years ago

But be aware that changing order of keys in some very rare cases may affect how your code works (I believe that's why autofix its not implemented in core eslint).

That's true, but that's due to bad quality of code, you should not confide in objects traversal order.

TSMMark commented 4 years ago

@leo-buneev This is one of the most common lint offenses my team gets on CI – you will save us so much time ❤️ Thanks so much

leo-buneev commented 4 years ago

Should be fixed in 1.1.0, let me know if there are any problems.

TSMMark commented 4 years ago

@leo-buneev amazing, thank you!

I just reintroduced this plugin into a medium-sized React codebase and I can confirm it now works as intended. Thanks again