Update to 10.2.0 results a huge amount of circular dependencies lint errors

[evtk]

Current Behavior

Circular dependencies suddenly occur after update to 10.2.0

Expected Behavior

Circular dependencies should not suddenly occur after update to 10.2.0. No issues on 10.1.0.

Steps to Reproduce

I'm afraid I can't share my repo. But I have 2 libraries:

shared/components shared/services

shared/components imports from shared/services, but shared/services doesn't import shared/components. I have double checked this.

Still after updating to 10.2.0 I suddenly get:

ERROR: 3:1 nx-enforce-module-boundaries Circular dependency between "shared-components" and "shared-services" detected.

This is just one example, I have loads of them.

I have opened up the runtime-lint-utils and added some console logging and for all I can conclude is that i goes looking up any other libs containing imports of shared-services and if those other libs are also importing shared-components, it resolves to true.

there is a funny twist in the code I noticed also:

export function isCircular(
  graph: ProjectGraph,
  sourceProject: ProjectGraphNode,
  targetProject: ProjectGraphNode
): boolean {
  if (!graph.nodes[targetProject.name]) return false;
  return isDependingOn(graph, targetProject.name, sourceProject.name);
}

function isDependingOn(
  graph: ProjectGraph,
  sourceProjectName: string,
  targetProjectName: string,
  done: { [projectName: string]: boolean } = {}
): 

see how targetProject and sourceProject are flipped as arguments?

Environment

nx : Not Found @nrwl/angular : 10.2.0 @nrwl/cli : 10.2.0 @nrwl/cypress : 10.2.0 @nrwl/eslint-plugin-nx : Not Found @nrwl/express : Not Found @nrwl/jest : 10.2.0 @nrwl/linter : Not Found @nrwl/nest : Not Found @nrwl/next : Not Found @nrwl/node : Not Found @nrwl/react : Not Found @nrwl/schematics : Not Found @nrwl/tao : 10.2.0 @nrwl/web : Not Found @nrwl/workspace : 10.2.0 typescript : 3.9.7

[FrozenPandaz]

Are you able to reproduce in a minimal example?

shared/components imports from shared/services, but shared/services doesn't import shared/components

Larger Circular dependencies can also occur such as A > B > C > A. If you view your dep-graph with nx dep-graph, can you see the circular dependency?

see how targetProject and sourceProject are flipped as arguments

This makes sense :wink:.

This import statement shows a dependency, A => B. Thus, to check for a circular dependency, we check if B => A.. or B => C => A.. etc.

[evtk]

Hi @FrozenPandaz thanks for your quick check-in! While I would love to supply a reproduction, this is something that is not easy to reproduce (and of course therefore, hard to pinpoint). In fact I have fairly large mono-repo, and this what the dep graph is looking like (and that is just a portion of it):

This makes it hard to track down the A -> B -> C -> A. What I do can confirm is that no A->B->A exists. With such a large number of libs, I can imagine that circular dependencies have grown. But, the linting should have taken effect to prevent this.

Now, running linting on a master branch of the repo does not give any errors (npm run lint shared-forms). Switching to a feature branch where I have updated to NX 10.2.0 and running an npm ci, will give me errors. This can mean 2 things:

A) there are in fact circular dependencies, but somehow the NX update has fixed a previous error with linting (I'm pretty screwed in this case :) B) there aren't circular dependencies, but after the update of NX linting is giving false positives

This import statement shows a dependency, A => B. Thus, to check for a circular dependency, we check if B => A.. or B => C => A.. etc.

Ahh dunno how I missed that, maybe I went of track due to naming of the function arguments :)

[evtk]

@FrozenPandaz I have investigated some more. Take a look at this picture (I had to mask 1 lib name, because it contains sensitive information let's call it lib shared/masked). I would definitely conclude that there is circular dependency here. Now if I search inside lib shared/forms I should find a reference to share/masked. Right? That reference does not exist! Searching inside folder shared/forms doesn't give any hits for shared/masked.

Now take a look at this picture, this is on 10.1.0:

Again I masked the lib: but it is the same as in the first image. No changes in code, but a completely different dep graph. In this setup: no circular dependencies.

So I'm starting to think there is something wrong with the dep-graph in 10.2.0. Are changes made to this?

[FrozenPandaz]

Awesome, those are very helpful details. What do the imports that the lint rule fails on look like? Can you also include the file path if possible?

[evtk]

sure (masked sensitive corporate information): /angular/libs/shared/forms/src/lib/validators/**/**.class.ts:3:1 ERROR: 3:1 nx-enforce-module-boundaries Circular dependency between "shared-forms" and "shared-webapis" detected

specified line:

import { ***Service } from '@***/shared/webapis';

Now watch this:

I should have expected an import from shared/forms into shared/webapis.

[evtk]

Also: I have served one of the applications using this actual validators class, and Angular is not giving me any circular dependency errors in the terminal.

[FrozenPandaz]

Neither dep-graph shows that web-apis depends on shared-forms so it makes sense that you did not find any results for that search.

The dep-graph does show however that shared-webapis depends on the masked lib, and also that the masked lib depends on the shared-components lib.

Here are the circular dependencies I see in the picture of the dep-graph in 10.2:

1. shared-components => masked => shared-components
2. masked => shared-forms => masked
3. shared-forms => shared-webapis => masked => shared-forms

I'm curious what the imports that create these dependencies are:

1. masked => shared-components
2. shared-forms => masked
3. shared-webapis => masked

[evtk]

Thanks for checking in again. I couldn't find any references to be honest, there simply are no imports. I'm sorry I have to 'mask' the specific lib, which obviously doesn't help.

I'm curious for your opinion on why the behavior has changed between 10.1.0 and 10.2.0. It seems you are investing on the possibility that circular references have always existed in my repo, but now suddenly have come 'alive' due to changes in 10.2.0?

[4kochi]

Hello @EvtK,

i recently had an issue with Circular Dependencies too. In my case i had an relative import to the environment.ts file in one of my test. So maybe you can also look if there are relative imports somewhere. But i also still use 10.1.0, so maybe you still have a different problem.

[evtk]

Hi @4kochi, thanks for checking in. We have linting enabled that blockes importing from relative file paths instead of the @library type of import. So I suppose this is not occuring, but I can of course double check that :).

You don't have issues on 10.2.0?

[4kochi]

No, i have updated today and everything went fine.

[x87]

Upgrading from Nx 9.5.1 to Nx 9.7.0 have resulted in a similar issue in our monorepo as well. All of a sudden new circular dependencies popped out of nowhere (not much, ~10 in total).

As with the example above our failing libs have references to shared libs, but shared libs does not seem to have any back-reference to those libs to produce a loop. Investigating.

Could 020d2e1 be a cause?

[x87]

I found that in our case the lint error was indeed caused by a circular dependency. But contrary to the confusing message the issue is not between two libs referencing each other (A->B, B->A), but with the one of the libs referencing itself with a named import (B->B). One of the shared libraries (say, libs/shared/util/index.ts) had an import like

import { x } from '@my-org/shared/util';

changing '@my-org/shared/util' to a relative import like

import { x } from '../interface;

seem to resolve the issue.

What would help if the lint error could display the correct library in case of self-references. I often find that triaging this particular lint error is very hard.

@EvtK check if shared-forms or shared-webapis libs don't self-reference as this is well might be the cause of your issue.

[evtk]

@x87 thanks for checking in! yes I will travel along the libs and see if they are self-referencing, I have had issues of this before. Hopefully it will fix it, although I still can't understand why this would suddenly pop-up on updating to 10.2.0. Will report back ASAP.

[evtk]

I have fixed all circular dependencies, reported with this tooling:

npx madge --circular --extensions ts ./

And I have gone through each lib, checking self-references. I have found some, mostly in spec files, and fixed them all. But still.. I get the same circular dependency errors.

I'm wondering if we are focussing to much on the circular dependency part of this, while there actually seems to be an issue with the deph-graph. I find it strange that a dep-graph would suddenly change between two NX versions without actually changing any functional code in my repo. @FrozenPandaz what is your opinion on this?

[x87]

@EvtK @FrozenPandaz A bug was indeed introduced with #3700. I noticed that some of the libraries that the linter reports as having circular dependencies target random libraries in the dependencies section of the nxdeps.json. Upon further inspection I came to this line:

https://github.com/nrwl/nx/blob/master/packages/workspace/src/core/target-project-locator.ts#L65

If I delete node_modules/.cache/nx/nxdeps.json and make a change in node_modules/@nrwl/workspace/src/core/target-project-locator.js as follows:

-        const resolvedModule = this.cache.has(normalizedImportExpr)
-           ? this.cache.get(normalizedImportExpr)
-            : typescript_1.resolveModuleByImport(normalizedImportExpr, filePath);
+        const resolvedModule = typescript_1.resolveModuleByImport(normalizedImportExpr, filePath);

the dep-graph is working fine again and does not have wrong references so the linter does not complain anymore.

@EvtK try to test the same with your project as well to see if it helps.

Up until this bug is fixed I wouldn't recommend to upgrade to v9.7.0+ and 10.2.0+ where this issue had been introduced.

[x87]

@FrozenPandaz I have identified one particular case when caching from #3700 would fail. One of our libraries (say, lib-a) has an import like:

lib-a:

import { x } from '.';

from now on, dep-graph assigns '.' to lib-a and caches this resolution. We also have a few other libraries that have the same import path:

lib-b:

import { y } from '.'

and it adds a dependency on lib-a for lib-b however they have nothing in common. As a result the linter using the malformed dep-graph is now producing false-positive errors where in reality they do not exist.

I don't know if there are other import patterns that could result in that same issue (../.., for example), but worth checking all possible cases.

[x87]

@FrozenPandaz ok, the issue is that this utility function https://github.com/nrwl/nx/blob/e9393805c175e9b7e5a94aca71fa654970ccb802/packages/workspace/src/utils/fileutils.ts#L114 does not recognize '.' as a valid relative path, resulting in broken logic starting here https://github.com/nrwl/nx/blob/be921083553bea0cbf6f87f3679489ce924917bf/packages/workspace/src/core/target-project-locator.ts#L50

'.' must not be a normalizedImportExpr.

So I think this should be the only use case that is left missing. @EvtK can you confirm if your code base has imports from '.'?

[evtk]

@x87 yes I plead guilty :)

I'm gonna repair all those references and report back if it solves the problem. Thanks for your thorough investigation!

[x87]

@EvtK you welcome :) don't forget to delete node_modules/.cache/nx/nxdeps.json to force Nx to reconstruct the dep-graph.

[evtk]

YES! I can report back that the issues are gone. Though I had to do a second run after fixing imports from '..' as well. But after this, the linting errors have disappeared!

I should be nice if we could have some additional linting that will throw lint errors when importing in this way ('..' and '.')!

[github-actions[bot]]

This issue has been closed for more than 30 days. If this issue is still occuring, please open a new issue with more recent context.