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

Consistent type import sorting #23

Closed
wants to merge 2 commits into from

Conversation

jasonneylon
Copy link

Pull Request Checklist

  • I have read the CONTRIBUTING document
  • Readme and changelog updates were made reflecting this PR's changes
  • Increase the project version number in package.json following Semantic Versioning

Changes Included

Sort type import into two groups, an external and built-in type group and an internal type group. this is to be consistent with the previous import/order rule and how non-type imports are sorted.

At the moment the types are sorted alphabetically. This is "by design' behavior in perfectionist and not a bug, see discussion in this bug report.

string[] — An import matching any of the values of the array will be marked as part of the group referenced by the key. The order of values in the array does not matter.
https://perfectionist.dev/rules/sort-imports

example before this rule change:

import type {
  customErrorCodes,
} from './schema';
import type { z } from 'zod';

After this rule change

import type { z } from 'zod';

import type {
  customErrorCodes
} from './schema';

@mangs
Copy link
Contributor

mangs commented Oct 7, 2024

Hi @jasonneylon. Thanks for the submission.

Traditionally type imports of all kinds have always been part of a single group, so I don't think this should be a change to the main config. However, if you want to setup your code style as you suggest in this PR, I have 2 ways we can accomplish that:

  1. Make the change locally. I assume you're trying to avoid this otherwise you wouldn't be here, which brings me to
  2. Create a team-specific config in this repo. I hadn't considered this before, but because this repo contains building blocks to construct and share what you need, we could have a team/* set of configs for each team so you can share those configs across your projects. Your team will have to assume support for those configs because they will inevitably drift out of date with the base configs, but if you're ok with that I can help you setup a team/blueberries/<whatever>.json config to help you get what you need. FYI I'll likely need your help when we do the ESLint v9 migration which will happen as soon as AirBNB's eslint config (a big dependency of ours) reaches v9 support (eslint v9 support airbnb/javascript#2961).

@jasonneylon
Copy link
Author

However, if you want to setup your code style as you suggest in this PR, I have 2 ways we can accomplish that:
Make the change locally. I assume you're trying to avoid this otherwise you wouldn't be here.

Nope it was mainly due to the change in the ordering that was introduced via adding Perfectionist sort-imports that I was hoping to address by this change. Looking at it more closely, however import/order didn't enforce an order of types. People on our team seemed to follow the external then internal order as that was established elsewhere in the rule. sort-imports does enforce alphabetical ordering however making the decision for us.

Traditionally type imports of all kinds have always been part of a single group, so I don't think this should be a change to the main config

Fair enough, it is not a big issue and not probably not worth creating more churn around.

@jasonneylon jasonneylon closed this Oct 8, 2024
@jasonneylon jasonneylon deleted the consistent-typescript-import-sorting branch October 8, 2024 12:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants