infra(unicorn): import-style

[ST-DDT]

Ref: #2439

• Check eslint-plugin-unicorn rules to enable/configure/disable #2439

---

Permanently disables the unicorn/import-style lint rule.

This rule defines how a specific module has to be imported aka import * as foo from 'bar' vs import { oof } from 'bar'.
Unfortunately, it has the opinion that you should import the entirety of node:path, because "they have similar functions, all likely to be utilized", which I disagree with. Since this doesn't apply to any non configured package, and the defaults are subjective (and wrong IMO), I think we should disable this.

[netlify]

✅ Deploy Preview for fakerjs ready!

Name	Link
🔨 Latest commit	4848bc2
🔍 Latest deploy log	https://app.netlify.com/sites/fakerjs/deploys/6658a55924ba7f0007a8a469
😎 Deploy Preview	https://deploy-preview-2901.fakerjs.dev
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.95%. Comparing base (9daff54) to head (4848bc2).

Additional details and impacted files

@@            Coverage Diff             @@
##             next    #2901      +/-   ##
==========================================
- Coverage   99.96%   99.95%   -0.01%     
==========================================
  Files        2986     2986              
  Lines      215926   215926              
  Branches      598      945     +347     
==========================================
- Hits       215843   215824      -19     
- Misses         83      102      +19     

see 2 files with indirect coverage changes

[Shinigami92]

Suggested change

	'unicorn/import-style': 'off', // subjective & doesn't do anything for us
	'unicorn/import-style': [
	'error',
	{ styles: { 'node:path': { named: true } } },
	],

[ST-DDT]

I tried that - see first commit - and I disagree.
The only thing we are using from that preset is node:path and there we don't use it and the other imports are totally unaffected.
We can talk about that in more detail, once I can talk again, If you would like.

Or let me ask the other way round:

• What is the benefit of enabling it that way?

[Shinigami92]

For example we use it here:

faker/scripts/apidocs/utils/paths.ts

Line 1 in b27e1a8

import { dirname, resolve } from 'node:path';

with that rule enabled, it prefers import { dirname, resolve } from 'node:path'; over import path from 'node:path';

with that rule set to off, anykind of syntax could be used

[ST-DDT]

Btw for that to be the case you have to set default to false.

But what about literally any other import in the entire project?

[Shinigami92]

If you would like, you can find a config for that
I would like to always prefer "named"-imports
I (personally) don't like the "default" of unicorn/import-style

[ST-DDT]

If you would like, you can find a config for that

Would you like to give it a try?

I would like to always prefer "named"-imports

Me too, but then there is the locales: https://github.com/faker-js/faker/blob/next/src/locales/cs_CZ/lorem/index.ts#L6

I (personally) don't like the "default" of unicorn/import-style

You can disable them completely with: extendDefaultStyles

[Shinigami92]

I currently have set me a restriction to not actively contribute code and work on code in Open Source this week / until I'm back from USA, so I do a semi-open-source vacation.

[Shinigami92]

@fisker is unicorn/import-style not simply configurable to just and only use named for every import possible? Or is that rule not intended for that usecase?

[fisker]

@fisker is unicorn/import-style not simply configurable to just and only use named for every import possible?

No, not possible.

Or is that rule not intended for that usecase?

You can send a feature request.

[fisker]

What is the benefit of enabling it that way?

It can be useful to avoid different style on the same module in one repo. You can choose any style that works and you like, personally I use named/namespace for node:path too.

Nothing we can do about it for now

[ST-DDT]

Since there is nothing we can do about it for now, I'll go ahead and merge this.
We can reconfigure this later once that feature is added.

EDIT: I don't know if you are going to create a PR, but if you do, you probably know more about how to configure it correctly anyway, so you can also create a new PR for the correct configuration.