Open vincerubinetti opened 1 year ago
Thanks for the feedback!
Could you share more details about:
Indeed, I'll try to get back to you with a minimum reproducible example soon. I was just wondering if you could spot any errors in my config above or your code just off the cuff.
Here is a minimal reproducible example:
First, cd
into /frontend
.
I want to format /src
, /e2e
, /unit
, but NOT /node_modules
.
This works, if I just want to format /src
:
npx format-imports --config ./import-sorter.json --log ./src
This part of the docs suggests you can specify multiple directories in a single command, but apparently only with --check
and not formatting:
npx format-imports --config ./import-sorter.json --log ./src ./e2e ./unit
As an alternative, with "exclude": ["node_modules"]
in the config file, I should be able to do this:
npx format-imports --config ./import-sorter.json --log ./
... but this still seems to scan node_modules
anyway. Maybe it doesn't actually format it, but it still takes a really long time and shouldn't be scanned.
So, to sum up:
I think this:
Should support multiple directories like this does:
AND
"exclude": ["node_modules"]
seems to not work. I'm not sure if it formats node_modules
, but it definitely scans all of it:
I don't think it should scan it, because that will take an enormous amount of time. The real project I'm using this in has 2x the packages in package.json
that the minimal reproducible example above has, and it's just impractical to way for this package to scan all of node_modules
every time.
I think you discussed 2 issues:
format-imports ./
is too slow and you suspected node_modules/ are formattedFor 1, I don't think node_modules/ are formatted after testing your repo on my laptop. The long time is because the tool needs to find out all files (including node_modules/) and check if they need process.
For 2, it's a design decision to support single directory because you can't specify -o
if there are multiple directories. The workaround is format-imports dir1/ && format-imports dir2/ && ...
.
For 2, it's a design decision to support single directory because you can't specify
-o
if there are multiple directories. The workaround isformat-imports dir1/ && format-imports dir2/ && ...
.
This is understandable.
For 1, I don't think _nodemodules/ are formatted after testing your repo on my laptop. The long time is because the tool needs to find out all files (including _nodemodules/) and check if they need process.
In my repo, it takes ~2 minutes to complete scanning node_modules
on a (very fast) M1 Macbook. I'd expect exclude
to ignore matches completely, like a .gitignore
, and not just ignore them for formatting.
I'm not sure how your code is selecting/filtering files and directories recursively, but you may want to consider using the glob
package for more conventional selecting/filtering behavior.
format-imports dir1/ && format-imports dir2/ && ...
This is fine but will get pretty lengthy and full of code duplication with a longer command: npx format-imports --config ./import-sorter.json --log dir
.
Here is the code for handling exclude
: https://github.com/daidodo/format-imports/blob/main/src/lib/config/index.ts#L39
Happy to receive PRs if you think it can be improved.
Thanks!
In my repo, it takes ~2 minutes to complete scanning node_modules on a (very fast) M1 Macbook. I'd expect exclude to ignore matches completely, like a .gitignore, and not just ignore them for formatting.
I'm in the same boat. Doing something like "exclude": ["node_modules"]
in my import-sorter.json
file does prevent the tool from reformatting the sources in node_modules
, but doesn't prevent the tool from scanning the directory, which takes O(minute).
For what it's worth, I ended up switching to a prettier plugin for this, @ianvs/prettier-plugin-sort-imports
. It works really well for me, integrating into a tool I'm already using for formatting and testing formatting, instead of a separate command I have to run. It also makes sense to me to be a Prettier plugin since it's just formatting and doesn't affect code functionality.
I appreciate the hard work that went into this library, but that plugin ended up being a smoother experience, both with integration and configuration. I think this library has more options though. Perhaps the author here could consider porting this to a Prettier plugin.
@vincerubinetti - Yeah, that makes sense. I use WebStorm, which has first-class support for Prettier, so making the switch to prettier-plugin-sort-imports was a bit of a no-brainer after I ran into the same issue that you did. Thanks for the tip.
Thanks for making this tool, it's great! This is more of a question or request for help. I would've made a discussion instead of an issue if they were enabled.
I'm basically trying to run this plugin recursively on a few select folders in my repo:
src
,unit
, ande2e
. Here's the setup I have:package.json
import-sorter.json
I've looked through the docs and tried a few things. If I append
./unit ./e2e
to the command, like the docs say I can do, I simply get errors likeCommand failed with exit code 1.
. I've also tried using theexclude
andexcludeGlob
config options to exclude the most time consuming directories,node_modules
anddist
, but no matter what variation I try, the plugin runs onnode_modules
and takes an impractical amount of time.One thing to possibly note is that I'm running this in a subdirectory in a monorepo,
/frontend
. I'm running the commands and everything relative to this directory, but in VS Code the project folder I have open is one level up, so maybe that is messing things up.Could you advise some things to try?