CODEWITHSUNDEEP
javascriptangulartypescript
Nicolas Gehlert
Nicolas Gehlert

Reputation: 3253

Are the circular dependency warnings in Angular really a problem?

For a while now Angular has supported a showCircularDependencies build flag (https://angular.io/guide/deprecations#angular-devkitbuild-angular) that alerts you circular dependencies inside classes.

But are those warnings really a problem? Let's take a look at an example

import {MyOtherService} from './my-other-service.ts';
@Injectable()
class MyService {
    constructor(private otherService: MyOtherService)
}
function calculate(): void {
    // some method that should be exported so it can be used without the service instance
}

export {MyService, calculate}

and file 2

import {calculate} from './my-service.ts'
@Injectable()
class MyOtherService {
    constructor() {
        calculate();
    }
}

This will now cause a warning my-service.ts -> my-other-service.ts -> my-service.ts

My question is, is this really problem? Because as far as I see JS is smart enough to only import the needed parts and is not causing a memory leak here.

And on a side note: for cases where it would cause problems for example MyOtherService injecting MyService in the constructor you will get hard build errors from Angular because it can't compile at all. (And also if you look at the documentation link above you will notice that this option to even show the warnings will be deprecated coming Angular 14)

Upvotes: 1

Views: 1663

Answers (1)

Raphael Jenni
Raphael Jenni

Reputation: 318

TL;DR

Extract the code into its own file and import it separately. Remove cyclic dependencies at all costs as it can lead to major problems.

Explanation

Personally, I think this is not a problem regarding if it works or not in the first place, but more an issue of code quality -> code coupling.

Let's take your example, but rename MyService to BaseService, and MyOtherService to InjectedService:

If we ignore the import and call to calculate for a moment, we are in the situation where BaseService depends on InjectedService. If you now change something in InjectedService, you always have to make sure, that nothing breaks in BaseService. But if you change something in BaseService, you don't have to worry about any implications for any other services.

If we now take the original example, you have a dependency in both directions (therefore the cyclic dependency). You are going to have to maintain always both services if you change anything to either one because stuff can break in both directions. At some point, you, or one of your coworkers, may forget to do that, and that's when bugs get introduced. Now the problem can get even worse, if the cyclic dependencies are indirect, meaning spanning over multiple files (>2; e.g. A -> B -> C -> A). Then it can get really complicated, and the smartness of JavaScripts module loading might be not enough anymore.

The solution to this problem is pretty straightforward. Extract the code you for calculate into its own file (it is a function anyway and has no connection to the class). Yes, it does result in an extra file with little content, but from a clean coding and maintainability standpoint, it is a much better option.

If you still think it is better the way you have it, you have the option to disable the warning in your angular.json file.

"defaults": {
    "apps": {
      "showCircularDependencies": false
    },
    ...
}

Upvotes: 1

Related Questions