Skip to content

Conversation

@cbraynor
Copy link
Contributor

These changes should remove the need to suppress the sort-imports lint in eslintrc.json.

Merge / deployment checklist

  • Confirm this change is backwards compatible with existing workflows.
  • Confirm the readme has been updated if necessary.

Copy link
Contributor

@robertbrignull robertbrignull left a comment

Choose a reason for hiding this comment

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

I note we also have "sort-imports": ["error", { "allowSeparatedGroups": true }]. What was the reason for adding that configuration? Do we want to remove that?

Previously the imports were sorted by those from external sources first and then those from local files, which doesn't appear to be a thing supported by the eslint rule. This is not necessarily a problem but I don't think we should try to emulate that with allowSeparatedGroups. If we want that we should look for a rule that can enforce it for us.

Overally I'm not hugely convinced by this rule as I can't see why sorting by the type of import (i.e. importing all members or just one) is helpful, especially when it means that the import sources are no longer alphabetical.

@cbraynor
Copy link
Contributor Author

cbraynor commented Oct 1, 2020

I switched to import/order instead which seems to be more aligned with what we want and has the added benefit of implementing the --fix functionality

@cbraynor cbraynor merged commit 4ff6c0d into main Oct 1, 2020
@cbraynor cbraynor deleted the cbraynor/fix206 branch October 1, 2020 11:22
@github-actions github-actions bot mentioned this pull request Oct 5, 2020
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