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
enable a few ESLint rules #1173
Conversation
|
4053316
to
92f09bf
Compare
Converting to draft while I tweak a few ESLint settings. @seek-oss/braid-maintainers how do we feel about @typescript-eslint/consistent-type-imports? |
I quite like the explicitness. Does that disallow single-lined multi-typed imports? Eg.
(where |
The rule auto-fixes import { fn, Foo } from 'foo'; to import type { Foo } from 'foo';
import { fn } from 'Foo'; But if you (re)write it as import { fn, type Foo } from 'foo'; that's also valid |
92f09bf
to
599d1d5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the thinking that we will touch many of these files twice now? Once to extract type imports to standalone imports, and a second to revert to inline when the plugin supports that preference?
Outside of that, a few other minor comments.
package.json
Outdated
@@ -59,7 +59,8 @@ | |||
"enquirer": "^2.3.6", | |||
"escape-string-regexp": "^4.0.0", | |||
"eslint": "^8.21.0", | |||
"eslint-config-seek": "10.0.0", | |||
"eslint-config-seek": "^10.1.0", | |||
"eslint-plugin-canonical": "^3.1.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add this now or later when typescript-eslint/typescript-eslint#4338 is done?
Have we considered this going into eslint-config-seek
or too opinionated you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If TypeScript 4.5 is a blocker, I would add it later; forgot to remove it now.
Same thing with adding it to eslint-config-seek
, aside from being (too) opinionated. If TS 4.5 stops people from upgrading, then it's not great.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me 😅
Rules enabled:
@typescript-eslint/consistent-type-imports
import/no-duplicates
import/no-relative-packages