infra(unicorn): import-style #2901
Conversation
ST-DDT
added
p: 1-normal
✅ Deploy Preview for fakerjs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## next #2901 +/- ##
==========================================
- Coverage 99.96% 99.96% -0.01%
==========================================
Files 2977 2977
Lines 215422 215422
Branches 950 604 -346
==========================================
- Hits 215351 215347 -4
- Misses 71 75 +4 |
| @@ -39,6 +39,7 @@ module.exports = defineConfig({ | |||
| 'prefer-exponentiation-operator': 'error', | |||
| 'prefer-template': 'error', | |||
|
|
|||
| 'unicorn/import-style': 'off', // subjective & doesn't do anything for us | |||
There was a problem hiding this comment.
| 'unicorn/import-style': 'off', // subjective & doesn't do anything for us | |
| 'unicorn/import-style': [ | |
| 'error', | |
| { styles: { 'node:path': { named: true } } }, | |
| ], |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
For example we use it here:
faker/scripts/apidocs/utils/paths.ts
Line 1 in b27e1a8
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
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
Ref: #2439
Permanently disables the
unicorn/import-stylelint rule.This rule defines how a specific module has to be imported aka
import * as foo from 'bar'vsimport { 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.