dusk-network / eslint-config

⚙️ Dusk ESLint configuration
Mozilla Public License 2.0
3 stars 12 forks source link

Add eslint-plugin-import-newlines for import consistency #12

Closed kieranhall closed 6 months ago

kieranhall commented 8 months ago

This plugin would enable us to set rules around the import section of our code and enforce more consistency.

ascartabelli commented 8 months ago

I'll look into this. Don't know why, but I thought it was something already in eslint itself and I've seen recently that it's not the case. I'd like it to follow the same rule as object destructuring.

Also about imports, as there are more plugins about them: I always avoided them as I pretty much saw them as pointless, although it's been a long time since I checked what's out there.

I said pointless, because actually I don't really care much about having them ordered in some specific way (this is what I saw those plugins doing), but rather I'd like a logic way of separating them. My choice is:

If we agree on this, or on something along these lines (@kieranhall @deuch13 @nortonandreev) I fear this should be a human rule and not a linter one...

kieranhall commented 8 months ago

My preference would be for:

I don't see the point in the blank lines. The plugin can ensure things like having a max number of items on one line, which I think is nice.

ascartabelli commented 8 months ago

I don't see the point in the blank lines. The plugin can ensure things like having a max number of items on one line, which I think is nice.

It is nice!

I was talking about another thing I wanted for imports and that I don't know if plugins consider at all.

kieranhall commented 8 months ago

With these OOTB rules, we could have the grouping (human rule) as you prefer by using allowSeparatedGroups and then have the imports be sorted alphabetically in their groups. https://eslint.org/docs/latest/rules/sort-imports.

Then I think it would be nice to have eslint-plugin-import-newlines to restrict the number of items to 2 and set a sensible max-len value too.

ascartabelli commented 8 months ago

With these OOTB rules, we could have the grouping (human rule) as you prefer by using allowSeparatedGroups and then have the imports be sorted alphabetically in their groups. https://eslint.org/docs/latest/rules/sort-imports.

We already have that: https://github.com/dusk-network/dusk-eslint-config/blob/main/js/style.js#L113

It's the bit where we have those three separate groups that I didn't find how to enforce back in the day. Maybe there's something new.

Then I think it would be nice to have eslint-plugin-import-newlines to restrict the number of items to 2 and set a sensible max-len value too.

On this I wholeheartedly agree, but the problem will arise with prettier. If I'm not wrong prettier considers a max-len only and not the number of imported members.

kieranhall commented 8 months ago

I was looking through the rules that are disabled by eslint-plugin-prettier and noticed that, when you filter out all the React and Vue rules, there aren't so many that we're using. I haven't searched for everything, but I did notice this post about eslint depreciating formatting rules.

I suppose the question is: what does prettier do when it finds imports spread over multiple lines that doesn't exceed it's max-len value?

deuch13 commented 6 months ago

I don't see the point in the blank lines. The plugin can ensure things like having a max number of items on one line, which I think is nice.

For me the blank lines would only be a personal preference as it would make it easier to differentiate between the imports.

kieranhall commented 6 months ago

I don't see the point in the blank lines. The plugin can ensure things like having a max number of items on one line, which I think is nice.

For me the blank lines would only be a personal preference as it would make it easier to differentiate between the imports.

If @nortonandreev agrees, we can do this by convention. The plugin doesn't do anything about removing newlines anyway, that's a Prettier thing, and I think it is preserved.