[web] Add eslint-plugin-simple-import-sort
#49368
Conversation
kiosion
added
do-not-merge
no-changelog
| // Side-effect imports (e.g. CSS) | ||
| ['^\u0000(?!.*\u0000$)'], | ||
| // Any 3rd-party imports (e.g. 'react', 'styled-components') | ||
| [`^(?![./])(?!(?:${excludedPackagesPattern})(?:/|$))(?!.*\u0000$).+`], |
There was a problem hiding this comment.
What's the deal with \u0000 in every import group? The diff that I posted, which was based on the default import groups, had it only in side effect imports.
There was a problem hiding this comment.
That represents a byte present at the end of type-only imports, so in order to keep those sorted to the bottom we need a negative lookahead at the end of previous import groups
| // Allow unused vars/args that start with an underscore | ||
| { | ||
| varsIgnorePattern: '^_', | ||
| argsIgnorePattern: '^_', | ||
| }, |
There was a problem hiding this comment.
👍, Bartosz was complaining about this recently.
|
At some point (maybe once you think it's ready to be reviewed) it'd be good to post about it on Slack to see what the rest of the team thinks about this. |
For sure, was planning to. Don't just want to open this big a PR (diff-wise) without asking some feedback first. |
kiosion
force-pushed
the
maxim/simple-import-sort
branch
2 times, most recently
from
fdf8f4c to
0853d45
Compare
* Add config for `eslint-plugin-simple-import-sort`: Serves same purpose as current `import/order` rule, but has better support for autofix, prevents newlines accumulating, and has 0 dependencies
kiosion
force-pushed
the
maxim/simple-import-sort
branch
from
0853d45 to
70acdd3
Compare
| import zlib from 'node:zlib'; | ||
|
|
||
| import tarFs from 'tar-fs'; |
There was a problem hiding this comment.
Node imports are grouped with 3rd-party libs.
This is what I found in the docs:
Each import is matched against all regexes on the from string. The import ends up at the regex with the longest match. In case of a tie, the first matching regex wins.
https://github.com/lydell/eslint-plugin-simple-import-sort?tab=readme-ov-file#custom-grouping
I think the current groups might work if we get rid of excludedPackagesPattern? The default sort groups have this:
// 3rd-party packages.
// Things that start with a letter (or digit or underscore), or `@` followed by a letter.
['^@?\\w'],
…so I believe we have to make sure we match just on the first letter (but idk how easy that is when type imports come into play).
There was a problem hiding this comment.
Have you seen this example? https://github.com/lydell/eslint-plugin-simple-import-sort/blob/66d84f742959bf68f4575b6ac5a65978ceb7cfbc/examples/.eslintrc.js#L136-L148
It has type imports last but doesn't use negative lookahead.
There was a problem hiding this comment.
Assuming that we want to enable some kind of rule which would automatically pluck out type imports from regular imports, I wonder if putting type imports last within each group would make sense?
There was a problem hiding this comment.
At the moment, no matter which option we pick wrt type imports isn't going to seem like a big deal. But down the road, assuming that their usage will be more common in the codebase, I wonder if it wouldn't look more tidy if type imports were within individual groups rather their own group.
Personally, when I look at imports, the most important information for me is whether the package is from the stdlib, or a 3rd-party lib, or maybe one of our packages. From that perspective, it'd make sense for type imports to be put within each particular group as well.
There was a problem hiding this comment.
Gotcha, that makes sense to me. Will take another look at this later, and see how things look grouping type imports at the end of groups.
Part of the issue we have currently is allowing importing types/interfaces alongside functions/enums/consts; I think enforcing using import type syntax would help clean things up as well.
There was a problem hiding this comment.
I figured out how to put type imports at the end of each group in #50619 (left a comment in the ESLint config explaining what's going on) and credited you in the commits under both OSS and ent PRs. I also based this on our updated ESLint 9 config.
eslint-plugin-simple-import-sort: Serves same purpose as currentimport/orderrule, but has better support for autofix, prevents newlines accumulating (currently, running import auto fix can lead to extra newlines being left in place), and has 0 dependencies