Skip to content

Conversation

keithamus
Copy link
Contributor

sort-imports seems to have a pretty inscrutable algorithm for how it sorts. It's not based on the import specifier, and seems to be based on bindings.

We see a lot of complaints about this rule:

  • The order is difficult to work out, making it high cognitive load to fix (or low cognitive load and multiple passes at running the lint rule until you do what it tells you)
  • It makes PR diffs messier as adding a binding can change the order required
  • It's not auto-fixable (which is exaggerated by the fact that the order is difficult to work out)
  • There seems to be no observable benefit to having imports sorted.

We added sort-imports in https://github.com/github/github/pull/51096 with a slew of other changes around import rules to catch bugs. However sort-imports didn't have good justification in isolation. Given the lack of justification originally, and given the amount of pain it causes today, I think it's a good idea to simply remove this rule.

@keithamus keithamus requested a review from a team as a code owner March 22, 2022 11:14
@keithamus keithamus requested review from koddsson and dgreif March 22, 2022 11:14
Copy link
Contributor

@koddsson koddsson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎩
😄 👍🏻

@keithamus
Copy link
Contributor Author

     🤠
  🤠🤠🤠
🤠  🤠  🤠
👍  🤠  👍
  🤠   🤠
  🤠   🤠
  👢   👢

@keithamus keithamus merged commit d882a8e into main Mar 22, 2022
@keithamus keithamus deleted the kill-sort-imports-with-fire branch March 22, 2022 11:20
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

Successfully merging this pull request may close these issues.

2 participants