Skip to content
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

Detect circular dependencies in the codebase #35303

Closed
mshustov opened this issue Apr 18, 2019 · 6 comments
Closed

Detect circular dependencies in the codebase #35303

mshustov opened this issue Apr 18, 2019 · 6 comments
Labels
discuss Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Team:Operations Team label for Operations Team

Comments

@mshustov
Copy link
Contributor

Problem

We faced a problem with the new babel preset typescript setup.
Modules with circular dependencies are processed by webpack without warnings, but sometimes in runtime exports aren't defined.
@mistic :

we found some of those cases when we move from typescript compiler to the babel preset typescript
which is not tolerant to circular dependencies at all, maybe that is the reason why it was working in the past and not now

The problem confuses developers and it is very hard to debug. Every project where I worked before had problems with circular deps and it led to long hours of debugging.

Suggestion

Since we are in a process of migration to the new platform, it looks like a good time to introduce additional checks to avoid future problems with cyclic deps.
I'd suggest adding an analyzer in our pipeline, preferably it's runnable with CLI interface. https://github.com/pahen/madge works well on my other projects so far.
If you want to test the output, run:

npx madge --circular src/

Ofc, it doesn't support our custom module resolution mechanism.
Want to hear from @elastic/kibana-operations

@mshustov mshustov added discuss Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Team:Operations Team label for Operations Team labels Apr 18, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-platform

@spalger
Copy link
Contributor

spalger commented Apr 18, 2019

I'm interested, as long as it doesn't get in the way of us doing things we need to!

@FrankHassanabad
Copy link
Contributor

FrankHassanabad commented May 8, 2019

We just ran into a bad circular deps issue on our end and what we did without full tooling in the meantime to detect and fix them was:

Install madge globally

npm -g install madge

Run a build

yarn build --no-oss --skip-os-packages

Then after the build go to the transpiled code:

cd ~/projects/kibana/build/kibana/x-pack/plugins

And run madge against a plugin or project

madge -c ${plugin_name}

That gave us a list in which we fixed.

@mshustov
Copy link
Contributor Author

looks like madge switched a resolution mechanism to https://github.com/dependents/node-dependency-tree, which supports webpack and tsconfig aliases out of the box ⭐️

@FrankHassanabad
Copy link
Contributor

We have been using a script which calls in madge within the plugin SIEM for the last ~2 months with great success if anyone wants to adapt it to their project. It's part of the build chain and fails any code which has a circular dep within it.

https://github.com/elastic/kibana/blob/master/x-pack/legacy/plugins/siem/scripts/check_circular_deps.js

https://github.com/elastic/kibana/blob/master/x-pack/legacy/plugins/siem/dev_tools/circular_deps/run_check_circular_deps_cli.js

@mshustov
Copy link
Contributor Author

closed in favour of #78162

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Team:Operations Team label for Operations Team
Projects
None yet
Development

No branches or pull requests

4 participants