imports in .spec files can trigger a circular dependency error on build

[dereekb]

Current Behavior

If I add an import to a .spec file from one library that is not technically part of the library but depends on another, I can induce a circular dependency bailout when building.

Expected Behavior

I expect the build to work successfully and not encounter this error, as the .spec files are not part of the library's final output. It seems like VS Code is building without issue since autocomplete is working.

Currently my alternative is to either package the tests in with the more dependent library, or create a third library that is strictly for carrying tests, which shouldn't be necessary. Also separating the .spec files from the files it tests will not be preferable.

Steps to Reproduce

As some background, I put a library together that had two sections of content, a "src" and an "test" folder that contained components strictly related to facilitating easier testing. If you've taken a look at Angular Material's project structure, there are testing projects along side each of their main components.

Here's what the dist I import from @angular/material looks like just so you have a good idea:

The easiest and probably best way I could guess for my Nx project was to create another project in the workspace, so I created a second project and gave it a -test suffix.

The issue arises when I need components from the testing components to test the main components. In my project, this is the "firebase" project along with the "firebase-test" project.

Now only the .spec files within the "firebase" project import from "firebase-test", but it still fails to build. I get the following error:

 >  NX   Could not execute firebase:build because it has a circular dependency

   firebase:build --> firebase-test:build --> firebase:build

To Reproduce:

• create two typescript projects, projects "a" and "a-test"
• have a file from project "a-test" import something from "a"
• have a .spec file from project "a" import something from "a-test"

Here's my project's repo where I first encountered this issue. I created a separate branch that better demonstrates what can trigger the issue.

https://github.com/dereekb/dbx-components/tree/chore/test-projects-example

https://github.com/dereekb/dbx-components/commit/53b4600d479d3ffb61613333aaa9b943f44ec33a

If you try and run nx build util-test or nx build util you will see the error. If you delete that test.spec.ts file it will behave properly. The issue also exists for nx build firebase, nx build firebase-server.

I think the example is well enough described, but I can also create a minimum repo with nrwl/nx-examples if needed. I recognize the above has a slightly different layout since the test projects are "nested" within the parent folder, but I don't believe this is causing an issue. The test folder is excluded from being built by the parent's tsconfig. I also verified that there weren't any hidden imports anywhere.

Failure Logs

 >  NX   Could not execute firebase:build because it has a circular dependency

   firebase:build --> firebase-test:build --> firebase:build

Environment

   Node : 16.13.2
   OS   : darwin arm64
   npm  : 8.5.2
   
   nx : 14.1.4
   @nrwl/angular : 14.1.4
   @nrwl/cypress : 14.1.4
   @nrwl/detox : Not Found
   @nrwl/devkit : Not Found
   @nrwl/eslint-plugin-nx : 14.1.4
   @nrwl/express : Not Found
   @nrwl/jest : 14.1.4
   @nrwl/js : 14.1.4
   @nrwl/linter : 14.1.4
   @nrwl/nest : 14.1.4
   @nrwl/next : Not Found
   @nrwl/node : 14.1.4
   @nrwl/nx-cloud : 14.0.3
   @nrwl/nx-plugin : Not Found
   @nrwl/react : Not Found
   @nrwl/react-native : Not Found
   @nrwl/schematics : Not Found
   @nrwl/storybook : 14.1.4
   @nrwl/web : Not Found
   @nrwl/workspace : 14.1.4
   typescript : 4.6.3
   rxjs : 7.5.4
   ---------------------------------------
   Community plugins:
   	 @ngrx/component-store: 13.0.2
   	 @ngrx/data: 13.0.2
   	 @ngrx/effects: 13.0.2
   	 @ngrx/entity: 13.0.2
   	 @ngrx/store: 13.0.2
   	 @ngx-formly/schematics: 6.0.0-next.9
   	 @jscutlery/semver: 2.23.4
   	 @ngrx/store-devtools: 13.0.2

Other Notes

It is being caused within the run-command task runner.

Errors occur here:

https://github.com/nrwl/nx/blob/53460849a277b518096536fe870efe564118601a/packages/nx/src/tasks-runner/run-command.ts#L396

It looks like the dependency generation doesn't discriminate what is needed for building something, but that level of context may not be easily determine automatically since build should be building for all cases, so to speak.

For dependency tree generation it is important that those projects are detected so the dependent projects can be built, but there should be a solution or configuration that can be used to ignore certain projects.

I think being able to manually exclude a project from being considered a dependency of another project would not only be the quickest solution but also the best solution.

[dereekb]

Looks like there are two other issues that are related to this:

#4547 #9645

I'm going to take a stab at a PR that will add a field to a project's configuration the ProjectGraphProjectNode type to exclude specific dependencies:

    "util": {
      "root": "packages/util",
      ...
      "targets": {
        ...
      },
      "excludeDependencies": ["util-test"],
      "tags": []
    },

Then make sure the project graph doesn't include these values.

[dereekb]

After digging around it turns out this functionality already exists, it's maybe just easy to overlook in the documentation or hard to find exactly when you run into the issue.

You can use the "implicitDependencies" configuration with an ! prefix:

      "implicitDependencies": ["!firebase-test"],

https://github.com/nrwl/nx/blob/05a9544806e8573bac5eef542a8e8c1b6115dc18/packages/nx/src/project-graph/build-dependencies/implicit-project-dependencies.ts#L12

Perhaps expanding a section somewhere concerning the error circular dependency error and using this as a remedy if the case is the same would be a good idea.

[parisholley]

running into this as well, will note that the ! functionality only works in certain circumstances, it doesn't help when the circular dependencies are transitive (more than one level deep)

[tmtron]

or create a third library that is strictly for carrying tests, which shouldn't be necessary

I agree with that: I'd also like to have the unit test in the same folder as the .spec file.
I'd like to exclude *.spec.ts and *.test.ts files from the enforce-module-boundaries check: i.e. implicitDependencies is not usable in that case

Update After deleting my project, and rebuilding everything the circular include warnings are gone.
Maybe it was some caching issue: before the issues occurred, I did some major refactoring: e.g. moved files to other libs, created new libs, renamed/deleted files, etc.

[JulioC]

I'm facing a similar issue due to dependencies used in *.stories.ts files. They are not required to run the project, but are added as dependencies in the published library.

I'd like to point that this issue might be related to #8096

[g3tr1ght]

I have a design related question to the NX team. Why does @nrwl/nx/enforce-module-boundaries rule whose purpose is "to ensure one can't import something that isn't meant to be imported in a particular location" also run irrelevant (from the usage perspective, not from the rule implementation perspective) circular dependency rule? And it specifically runs in a way which doesn't allow to configure/disable circular dependency checks. You somehow can configure a workaround but if you have a transitive circular dependency - you're doomed. So your only way around is to disable this highly valuable feature completely. Is there a justification for this particular design decision which explains why two distinct rules implemented as one make developers life easier (as that's what NX was created for ultimately)?

Let me unwrap it for you why in my opinion it is quite sizable culprit for "large-scale development" (this what NX was made for, right?).

Firstly, it violates a few SOLID principles, namely:

• Single responsibility principle: two distinct processes are being run as a single operation and failure in one of them makes the entire operation fail, whereas these processes have their own specific purpose and provide disjoint value on their own, so they can/should exist separately;
• Open–closed principle: I think it's clear to everyone that the only way for us (developers) to use/extend this rule (given its' current design) is via direct modification; which is what we did with multiple NX features, e.g.:
  • webpack config inflexibility due to hardcoding,
  • babel config inflexibility due to hardcoding,
  • inability to support multiple configurations (basic Angular CLI feature which NX is built on top of), 2.5 yo issue: https://github.com/nrwl/nx/issues/2839

We have fairly large codebase and are in a process of migrating legacy applications to the monorepo built with NX. Unless I'm missing anything, please correct me if I am, non-trivial migrations (combined with a rewrite) imply quite a lot of pre-existing issues in the objects under migration and tooling that (as it is currently stated on NX intro page) "has two main goals: Speed up your existing workflow with minimum effort. and Provide a first-rate developer experience no matter the size of the repo." is expected to support such non-trivial but very much expected activities.

As of today our NX monorepo is stitched with a number of workarounds and I feel like it's going mostly against us. I also don't see NX being able to support multi-framework monorepo no matter what is stated in the docs as it's struggling to support just a single-framework monorepo. I must admit that our use-case is not trivial but why would one use a monorepo for trivial applications/libraries, just have a repo per app and a couple repos for libraries, right? At the end of the day separate repos is by far more convenient structure from the releases schedule perspective and is the main bottleneck for monorepos (merge conflicts hell and heavyweight communication between a few dozen UI, back-end, QA teams).

Going back to the circular dependencies and module boundaries problem, we can't and shouldn't expect all code (how about migrations, right?) to be flawless and there must be flexibility in terms of configuration to aid developers in their valuable effort, otherwise you are forced into two distinct states - everything is correct or everything is broken; and there is no third state - mostly correct and moving towards full correctness.

Given NX team strong opinion on circular dependencies situation, our only logical way to avoid that is to lump all modules together in a single library and use precise import specifiers instead of index.ts barrels, but now a monorepo reduced to just "a repo". Nobody wants to create meaningless sub-libraries (which don't make sense on their own) just to extract some shared code out of multiple logically created libraries. Say, I have users, comments, scheduler libraries encapsulating logically relevant code, which could depend on each other. To avoid circular dependencies (especially with auxiliary modules like tests it's unavoidable), I now need to create the following : users, comments, scheduler, users-comments-shared, users-scheduler-shared, comments-scheduler-shared, users-comments-scheduler-shared, users-comments-shared-types, users-comments-shared-constants, comments-scheduler-shared-types, how-many-more?? This design decision limiting the structure flexibility has a polynomial complexity in terms of scalability** and is harmful in real large-scale development environment.

PS. No bashing here, it's actually amazing that NX team is tirelessly working on OSS project that I believe improved dev productivity, so far I've seen it being successful in setups where most of the code lives inside each app library while there's not much sharing, and what I'd love to happen is NX being able to support non-trivial, mid-to-large projects with decent shareability, so nobody will even look around for the alternatives like Turborepo. Thanks NX team!

[adina-ahmad]

For me !excluded-dependency wasn't working and happening with a lot others in community. I solved this by removing dependency configurations from the dependent module. E.g. In project.json, targets.build.dependsOn

From:

"dependsOn": [
        {
          "projects": "dependencies",
          "target": "build",
          "params": "forward"
        }
      ]

To:

"dependsOn": []

And adding the said dependencies to main nx.json like so:

  "targetDependencies": {
    "build": [
      {
        "target": "build",
        "projects": "dependencies"
      }
    ]
  }

[fknop]

After digging around it turns out this functionality already exists, it's maybe just easy to overlook in the documentation or hard to find exactly when you run into the issue.

You can use the "implicitDependencies" configuration with an ! prefix:

      "implicitDependencies": ["!firebase-test"],

https://github.com/nrwl/nx/blob/05a9544806e8573bac5eef542a8e8c1b6115dc18/packages/nx/src/project-graph/build-dependencies/implicit-project-dependencies.ts#L12

Perhaps expanding a section somewhere concerning the error circular dependency error and using this as a remedy if the case is the same would be a good idea.

This doesn't seem to work anymore in 15.4. https://github.com/nrwl/nx/blob/a806feed0eb8572e51262de9b08910bedb21f07c/packages/nx/src/project-graph/build-nodes/workspace-projects.ts#L157 This function seems to be removing project starting with ! from the implicit dependency array before going into the function linked in your comment.

[joewIST]

Also reporting the same issue with excluded dependencies. We have files imported into .stories.ts files, but the dependency graph is includes these. It was previously possible to use the "!" syntax, but this is not working.

Has anyone found a workaround?

[asilvadesigns]

Same, I cannot use the "!" syntax.

[peter-afs]

This -- combined with https://github.com/nrwl/nx/issues/14377 -- has caused me to need to disable several tests from CI. This is a serious issue.

[github-actions[bot]]

This issue has been automatically marked as stale because it hasn't had any recent activity. It will be closed in 14 days if no further activity occurs. If we missed this issue please reply to keep it active. Thanks for being a part of the Nx community! 🙏

[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.