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

Consider using an eslint import plugin instead of import/order #647

Closed
danvk opened this issue Jul 23, 2020 · 6 comments · Fixed by #1402
Closed

Consider using an eslint import plugin instead of import/order #647

danvk opened this issue Jul 23, 2020 · 6 comments · Fixed by #1402
Labels
area: incorrect converter Rule converter with incomplete, incorrect, or invalid rule names and/or arguments status: accepting prs Please, send in a PR to resolve this! ✨

Comments

@danvk
Copy link
Contributor

danvk commented Jul 23, 2020

🚀 Feature Request

The script swaps in import/order for tslint's ordered-imports. I spent quite a while trying to configure it to sort things like tslint's rule did, before discovering eslint-plugin-simple-import-sort, which seems much closer to the old rule (and also much easier to configure!).

Existing Behavior

ordered-imports becomes import/order.

Change Proposal

ordered-imports becomes eslint-plugin-simple-import-sort

I haven't taken the time to check how well this would work for tslint.json settings beyond the ones I use, but for me simple-import-sort was fantastic!

@KingDarBoja
Copy link
Collaborator

Hi @danvk

In this case, we are basing our rule converters (hence the ESLint outputs) to the ones defined at typescript-eslint ROADMAP.

@JoshuaKGoldberg JoshuaKGoldberg added question Further information is requested status: in discussion This issue needs more discussion before code changes labels Jul 25, 2020
@JoshuaKGoldberg
Copy link
Member

Heh, thanks for the link to eslint-plugin-simple-import-sort, that's a very appealing package...

For now, as KingDarBoja said, I think we are a little bound here to go with the typescript-eslint roadmap for the canonical rule replacements... however, IMO it really wouldn't be the end of the world if either:

  • We bucked the trend and recommended a much closer rule
  • The roadmap switched to the much closer rule

I don't suppose you have any thoughts @bradzacher?

@bradzacher
Copy link
Member

bradzacher commented Sep 8, 2020

🤷 I have no opinions about the "roadmap" file - the file was pieced together by people who aren't me over the years!

I've never used TSLint myself so I just defer to the community's guidance here.
If there's a better rule feel free to update the file appropriately.

@KingDarBoja
Copy link
Collaborator

If that's the case, we could switch to the much closer plugin as it is easier for users to configure it like TSLint equivalent. I don't think community will be annoyed by this small change as this is only 1 rule being changed.

@JoshuaKGoldberg JoshuaKGoldberg added area: incorrect converter Rule converter with incomplete, incorrect, or invalid rule names and/or arguments status: accepting prs Please, send in a PR to resolve this! ✨ and removed question Further information is requested status: in discussion This issue needs more discussion before code changes labels Sep 8, 2020
@JoshuaKGoldberg JoshuaKGoldberg self-assigned this Oct 2, 2020
@JoshuaKGoldberg JoshuaKGoldberg removed their assignment Nov 8, 2020
@hyperupcall
Copy link
Contributor

hyperupcall commented Feb 28, 2022

Hmm, I've been playing around with this and it seems eslint-plugin-import could be a better fit instead of eslint-plugin-simple-import-sort. The former has many options that line up with TSLint, including alphabetically, case-insensitivity, and grouping. eslint-plugin-simple-import-sort only has one option, groups, which means many options will be dropped during the conversion. There is also the fact that eslint-plugin-import seems to be more popular.

As others have mentioned, if we are deferring rules to an external package, we might as well make them as close as possible to TSLint. I would like to create a PR with either option ^w^, but I just wanted to make sure we make the right choice before I work on it

@JoshuaKGoldberg
Copy link
Member

Hmm, that makes sense - you're right @hyperupcall. I'd love a PR for eslint-plugin-import then!

@JoshuaKGoldberg JoshuaKGoldberg changed the title Consider using eslint-plugin-simple-import-sort instead of import/order Consider using an eslint import plugin instead of import/order Mar 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: incorrect converter Rule converter with incomplete, incorrect, or invalid rule names and/or arguments status: accepting prs Please, send in a PR to resolve this! ✨
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants