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

Rule suggestion: Forbid cyclic dependencies #1119

Open
dtinth opened this issue Apr 27, 2018 · 9 comments

Comments

@dtinth
Copy link

commented Apr 27, 2018

I looked in the Issues board for standard and eslint-config-standard project but did not find anything related to this, so I decided to start a new issue.

While circular dependencies are useful in some cases, based on my experience, it more often than not led to very tightly coupled modules, which are very painful to refactor. In my opinion, it would be a great idea if dependency cycles could be prevented from the start.

Moreover, in a large project with 1,000’s of files and multiple developers, without always checking, it’s easy even for very experienced programmers to inadvertently create a dependency cycle, leading to errors such as “MyModule.myFunction is not a function,” caused by MyModule being partially executed due to being involved in a dependency cycle.

Prior art

  • Go language does not support cyclic dependencies at all.

    The lack of circular imports causes occasional annoyance but keeps the tree clean, forcing a clear demarcation between packages. As with many of the design decisions in Go, it forces the programmer to think earlier about a larger-scale issue (in this case, package boundaries) that if left until later may never be addressed satisfactorily.

    Having debugged cyclic dependency problems several time in a large project, I used to say, “If a project is written in Go, we could hire any Go programmer and they will never cause a single dependency cycle, no matter how junior they are!”

  • Airbnb’s style also forbids dependency cycles.

Implementation

It can be added by using import/no-cycle rule (ES6 import) or dependencies/no-cycle rule (require / import / export).

Impact

Cons:

  • Annoyances.
  • Harder to adopt in project that already contains cyclic deps.
  • Harder to upgrade if project already contains cyclic deps.
  • Cyclic deps can sometimes be useful.

Pros:

  • In greenfield projects, we can just install standard and don’t have to worry about someone create cyclic deps, without having to jump through hoops and switch to standardx or eject to ESLint.
  • So that “This module helps hold our code to an even higher standard of quality.”
@feross

This comment has been minimized.

Copy link
Member

commented Apr 27, 2018

This sounds great. Happy to try enabling this in a future version of standard.

@feross feross added this to the standard v12 milestone Apr 27, 2018

@stale

This comment has been minimized.

Copy link

commented Jul 27, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale label Jul 27, 2018

@michael-ciniawsky

This comment has been minimized.

Copy link

commented Jul 27, 2018

...

@stale stale bot removed the stale label Jul 27, 2018

@feross feross modified the milestones: standard v12, standard v13 Aug 28, 2018

@stale

This comment has been minimized.

Copy link

commented Nov 26, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale label Nov 26, 2018

@dtinth

This comment has been minimized.

Copy link
Author

commented Nov 27, 2018

@stale stale bot removed the stale label Nov 27, 2018

@stale

This comment has been minimized.

Copy link

commented Feb 25, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale label Feb 25, 2019

@LinusU

This comment has been minimized.

Copy link
Member

commented Feb 25, 2019

Would still like to see this ☺️

If anyone wants to make a PR against the config repo, and compile a report of how many repositories this would break, that would be awesome 👍

@stale stale bot removed the stale label Feb 25, 2019

@stale

This comment has been minimized.

Copy link

commented May 26, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale label May 26, 2019

@LinusU

This comment has been minimized.

Copy link
Member

commented May 28, 2019

Would still like to see this ☺️

If anyone wants to make a PR against the config repo, and compile a report of how many repositories this would break, that would be awesome 👍

@stale stale bot removed the stale label May 28, 2019

@stale stale bot removed the stale label May 28, 2019

@feross feross modified the milestones: standard v13, standard v14 Jul 5, 2019

@feross feross modified the milestones: standard v14, standard v15 Aug 12, 2019

@feross feross added the enhancement label Aug 12, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants
You can’t perform that action at this time.