[import/no-cycle] do not check node_modules

[sveyret]

I've got projects with only a very few small files, which I have hard time working on because of no-cycle being too slow (99% of the full eslint time). I wanted to speed-up this time, but I did not want to use maxDepth because I may have some more complex projects and those are the ones where I'd mostly create an unwanted cycle.

While benchmarking the code, I found that there doesn't seem to be a cache, or the cache is not working. For my project, most of my files import the same very big module (typescript) and this module is analyzed at each import, instead of only at the first time.

But an easy correction which could also improve the speed would be to ignore external modules: there is very, very little chance that a dependency of my project references a file in my project.

For the moment, this should not be a big problem (except for my big typescript dependency), because most dependencies are transpiled into CommonJS and use require instead of import. But if one day, issue #1056 is corrected, or because external dependencies use more and more import, things will become worse.

I made a quick test by directly modifying the code and the time spent by no-cycle fell from more than 2mn to less than 30s.

I can submit a PR if you want. I don't know if it could be considered an issue (the correction would be a breaking change) or if it the behavior should be controlled by an option of no-cycle.

[ljharb]

We have an ExportMap, so we should be able to cache things using that in the no-cycle rule.

We definitely can't ignore external modules; a circular dependency is quite possible via one.

[sveyret]

Hi @ljharb, and thank you for your answer.

We have an ExportMap, so we should be able to cache things using that in the no-cycle rule.

no-cycle is using ExportMap indeed, but I don't know why, each time it looks at the typescript dependency, it takes a very long time. Maybe the cache is not working as expected? If someone wants to investigate further on this problem, he can benchmark this small project:

https://github.com/slune-org/ts-transform-auto-require

We definitely can't ignore external modules; a circular dependency is quite possible via one.

I can't imagine how it can be possible, but no problem, I can add an option to no-cycle. For example:

"ignoreExternal": true

which would be considered false if not specified.

Would you accept a PR for this?

[ljharb]

Given that "external" can include many things, including npm linked packages, I'm hesitant to allow it to be ignored.

[sveyret]

Sorry to insist on this @ljharb, but I still don't understand what makes you hesitate.

The purpose of no-cycle is not to detect cycles inside external packages. It is the responsibility of the external package.

You may need to inspect external dependencies if you have a monorepo with cyclic links between projects (which is detected at least by yarn, probably also by other package managers), but this will not be the main use case of no-cycle. Am I wrong?

So for the majority of the usages, having such an option would greatly improve the no-cycle execution time without any inconvenient. You could see this option a little bit like a smart maxDepth option.

If you prefer, I could implement an option which takes a regexp instead:

"ignore": "\/node_modules\/"

which would be undefined by default, and would allow not to check cycles in files with full path + name matching the regexp.

Please come back to me when you have decided weather you want me to do a PR.

[ljharb]

The purpose is to detect cycles anywhere, some of which might not be exposed within the external package alone.

[silverwind]

I think I'd also like a option to exclude third-party code in the no-cycle rule, just to speed up the rule in general. Either a glob pattern, path regexp or static option to ignore any imports starting with bare names would be fine.

[ljharb]

I've thought about it - I'd be willing to accept a PR with an option that specifically ignores cycles in "external" module (https://github.com/benmosher/eslint-plugin-import#importexternal-module-folders)

[sveyret]

I have no time immediately, but I'll do it within a week or two if no one else takes it before…

[sveyret]

Ready for review: #1681