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

Add sorting in requires/imports rule #41

Closed
diogotorres97 opened this issue Jun 7, 2021 · 8 comments
Closed

Add sorting in requires/imports rule #41

diogotorres97 opened this issue Jun 7, 2021 · 8 comments

Comments

@diogotorres97
Copy link
Contributor

https://www.npmjs.com/package/eslint-plugin-require-sort

@satazor
Copy link
Contributor

satazor commented Nov 4, 2021

@Americas
Copy link
Collaborator

Americas commented Nov 4, 2021

@satazor that's all well and good, we can look to change the one we already use for import/export lines. But what we need is a sort rule for require lines. Maybe I didn't dive deep enough, but it doesn't seem to support that.

@satazor
Copy link
Contributor

satazor commented Nov 4, 2021

But what we need is a sort rule for require lines

If you mean require statements at the top of files, yes this plugin supports it. It supports CJS and ESM. Check these test sample files:

At the moment the plugin does not auto-fix ordering. There was a discussion about it here import-js/eslint-plugin-import#2154 but the maintainer seems to disagree supporting it. However, having linting errors right when you are coding in your editor is pretty good and sufficient.

Let me know if I'm misunderstanding what you mean.

@Americas
Copy link
Collaborator

Americas commented Nov 4, 2021

Alright then! 😄

But there doesn't seem to exist an option to group just destructured requires, I think that is a dealbreaker.

The biggest problem here is that senior management has no time for simple things like these, but insist on making the decisions 🤷 so nothing changes 😢

@satazor
Copy link
Contributor

satazor commented Nov 4, 2021

But there doesn't seem to exist an option to group just destructured requires, I think that is a dealbreaker.

You want top put together destructured requires within the same group (e.g.: within nodejs native requires)? I think that's going to far, do we really need that for code to become more readable? We always should ask this ourselves so that our personal preference weights less.

The biggest problem here is that senior management has no time for simple things like these, but insist on making the decisions 🤷 so nothing changes 😢

@nunofgs perhaps the RFC process could help here? It would contain guidelines on how the RFC should / could be approved or declined.

@Americas
Copy link
Collaborator

Americas commented Nov 5, 2021

I'm not saying it's a dealbreaker to me 😄 I'm happy to abide by any guideline if that means less work during coding, and less hassle when reviewing! If on top of that the code is more readable, great!

@waldyrious
Copy link
Contributor

Should this issue be considered resolved now that #53 has been merged?

@satazor
Copy link
Contributor

satazor commented Mar 1, 2022

Yes we can close this. Thanks for pointing it out @waldyrious

@satazor satazor closed this as completed Mar 1, 2022
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

4 participants