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

Do not move properties across spread #3

Closed
gajus opened this issue Jan 1, 2019 · 8 comments
Closed

Do not move properties across spread #3

gajus opened this issue Jan 1, 2019 · 8 comments

Comments

@gajus
Copy link

gajus commented Jan 1, 2019

See:

eslint/eslint#10715 (comment)

gajus added a commit to gajus/eslint-config-canonical that referenced this issue Jan 1, 2019
@gajus gajus changed the title Do not more properties across spread Do not move properties across spread Jul 27, 2019
@hcharley
Copy link

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

@gajus
Copy link
Author

gajus commented Oct 29, 2019

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
Copy link

TSMMark commented Nov 11, 2019

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
Copy link
Owner

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
Copy link

piranna commented Nov 12, 2019

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
Copy link

TSMMark commented Nov 12, 2019

@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 added a commit that referenced this issue Nov 13, 2019
@leo-buneev
Copy link
Owner

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

gajus added a commit to gajus/eslint-config-canonical that referenced this issue Nov 13, 2019
Earlier this was disabled due to lack of support for spreads.
This now has been fixed in leo-buneev/eslint-plugin-sort-keys-fix#3
@TSMMark
Copy link

TSMMark commented Nov 13, 2019

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants