Support for circular dependency checks

[marcghorayeb]

Hello,
👍 for the great repo. I saw the roadmap and was wondering if you had progressed on the circular dependency check?
Thanks in advance,

[sverweij]

Thanks for the kind words 😊

I haven't started on a circular dependency check yet - was waiting for a demand for it.

[marcghorayeb]

I work on a large codebase mixing up JS/Coffee/TS and we are starting to see a lot of problems due to circular dependencies, making a tool to check these quite important to us.
I haven't any clue as to how complicated it would be to develop such a check but I'm ready to help out if you're open to PRs?

[sverweij]

Yesterday I've given some thoughts on how to implement this. Today I have a version running on my machine that correctly detects circular dependencies. It needs some polishing; I will try to publish coming week.

Are you willing to beta test it on your code base and provide me with feedback?

(Generally PR's are welcome b.t.w.)

[marcghorayeb]

That was quick! Thanks a lot.
Sure, will check out the PR against our codebase, anything specific you want me to test out first?

[sverweij]

Cool!

Nothing specific to test first - just want to know whether it works as you expect it to (accuracy, usability, speed).

B.t.w. how do (/ will) you use dependency-cruiser in your development process?

[marcghorayeb]

Did a quick check, it seems to work as expected 👍 I'll do some more testing tomorrow.

At first the accuracy seems good. Maybe some more info would be nice when outputting to the console (what path was used to see the circular?). The speed seems acceptable, we have around 2000 files and it went through it in approximately 10 seconds (no transpilers, going through the transpiled code, so only JS files in it).

At first we only want to show some warnings when the branches are being built on our CI tool (Atlassian Bamboo) and later on we might enforce no circular dependencies. At the very least, we know of a couple of files that are quite sensitive to any dependency changes and we might enforce no more than X circulars on a given file.

[sverweij]

Thanks. This is useful feedback!

• I'll see if I can add the offending path to the output - doesn't seem to be very complex to do and it'd be massively useful. Thanks for that suggestion! I'll include it with the publish later this week if possible.
• It might be possible to help you enforce to not have > X circulars on a file - but I'll have to think about how to implement it.

I've been using this version against a warty codebase as well and found the speed to be in need of improvement (~20s). Although acceptable for some scenarios (like on a CI), it isn't ideal for others (e.g. in a pre-push trigger) - I have some ideas on how to improve it but it'll be in a separate task.

[sverweij]

Hi @marcghorayeb I've published v2.2.0 - it seems stable enough. It includes the basic dependency-detection and it is a little faster than the branch version you tested (thanks again!).

I've put separate items on the roadmap for including the offending path in the output and for enforcing to not have > X circulars on a file.

If you have more suggestions/ results based on usage on your codebase let me know

[marcghorayeb]

That's great 👍 I'll test the new version today and will let you know how it goes, thanks for the quick update.