Disable Circular Dependency Check

dcrdev commented 1 year ago

Description

Our project is composed of a core, testing and several plugin packages, our testing package is utilized by our e2e tests to spin up a test server, the package has a dependency on core. Our core package has its own e2e tests and these utilize the testing package, within our core package we have a dev dependency on the testing package.

Currently I cannot build the solution because it results in Nx complaining about the circular dependency, it seems like a perfectly valid setup to me; we don't have a build time dependency on testing in core, we have a dependency in our tests only.

I have seen the various issues related to this and there are two proposed suggestions:

Use explicit dependsOn directives with the !package-name syntax This does not work at all in my situation
Move tests to a seperate package Would be impractical to do this, because not only would core require an e2e testing package, so would all of our plugins and their siblings.

It would be helpful if there were a way to bypass this check somehow

Motivation

It's widely requested, see: #2570, #10290, #9645

As of yet none of those discussions talk about a use case where you have a plugin ecosystem for which it's extremely beneficial to have a standardised framework for writing tests.

Suggested Implementation

A couple of possible implementations:

Opt-in flag to disable the check
Config directive to exclude certain paths from this check
Ability to not consider tests to be part of the overall graph
Ignore dev dependencies

Alternate Implementations

Another implementation might be to ignore circular dependencies when analyze source files is set to false.

stevenxu-db commented 1 year ago

An option to do this would be appreciated. Bazel solves this by building the dep graph per target. There's no such thing as a project (package in Bazel) having dep edges that all targets must use, which appears to be the case in nx. Is there a way today to express a graph for a project containing 3 workspaces:

A linter (e.g. an ESLint wrapper)
A tester (e.g. a Jest wrapper)
An app

None of the three packages have any build deps, but all three depend on the linter being built to lint, and all three depend on the tester being built to test. Is there a simple way to express this in nx today?

g3tr1ght commented 1 year ago

It would be great to split nx-enforce-module-boundaries into two eslint rules:

one that checks for circular dependencies (there already exists an import/no-cycle rule in eslint-plugin-import, so an additional NX rule seems like a redundancy)
one that actually enforces module boundaries with a deep/shallow setting to (dis-)allow for transitive libraries consideration (we don't want to specify all possible downstream tags, this setting would allow for sensible implicit tags inheritance).

Example: libA, tag scope:A, // imports B libB, tag scope:B, // imports C libC, tag scope:C, rules: scope:A depends on scope:B, scope:B depends on scope:C I would expect scope:A depends on scope:C to be inferred.

Given NX team opinion on circular dependencies situation, example provided in the official docs (https://nx.dev/core-features/enforce-project-boundaries) actually does promote circular dependencies:

        {
          "sourceTag": "scope:shared",
          "onlyDependOnLibsWithTags": ["scope:shared"]
        },
        {
          "sourceTag": "scope:admin",
          "onlyDependOnLibsWithTags": ["scope:shared", "scope:admin"]
        },
        {
          "sourceTag": "scope:client",
          "onlyDependOnLibsWithTags": ["scope:shared", "scope:client"]
        }

At a first glance one could think that even self-import is allowed. We obviously want to prevent self-imports (via both library alias and via relative import specifier) but circular imports between two libraries must be allowed without disabling the entire valuable rule. Single barrel exports file per each library and imports from the barrel is what amplifies circular dependencies problem. Also, NX project depends on itself, NX prior versions are the dependencies for the current NX version; this seems like a clear circular dependency by nature. Also, it seems that NX does violate its own philosophy: https://github.com/nrwl/nx/blob/master/packages/angular/src/generators/application/lib/create-files.ts#L3. I'm not saying that NX is doing anything wrong in this particular case and I'm not saying that circular dependencies must be encouraged, my point is, there's objective and subjective things (especially linting), not every rule is absolute, and circular dependencies are not strictly bad, they sometimes aren't avoidable (top-level barrel imports) unless one is willing to use numerous meaningless workarounds.

I hope this makes sense to NX team. We would highly appreciate enhancements in that quite critical (for large projects) space.

angelfraga commented 1 year ago

Hi , I am also facing similar issue, some core lib which test files import stuff from other libs.

I think the best approach would be splitting up the core libs into two projects, one core lib, another core-testing. That would allow setting tags properly for testing libs/apps eg like when using e2e app.

But also I think it might be a possible solution. I guess "excludedFiles" currently does affect filtering out files in the lib being lint but it does not filter the files in the graph for circular dependency checks.

For example with "excludedFiles": ["*.spec.ts"]

libs/other/src/lib/some.service.ts

libs/core/src/lib/some.spec.ts (imports {Service} from @project/other)

When running lint for core , spec file will be skip and lint finishes successfully when running lint for other, the circular dependency is found.

What do you think if the "excludedFiles" filter out the files also in other projects ?

angelfraga commented 1 year ago

Hi @dcrdev maybe I have a temporal solution for you.

While in my team we've decided to go for moving tests to another project, in your case you could use an alias instead of a new project. Then export everything from the conflictive package in a ts file at the root folder.

eg tsconfig.base.json

{
  "paths": { 
      ...
       "@my-workspace/lib-a/testing": "./testing.ts"
      ...
   }
}

tsconfig.json new file for giving IDE support

{
  "extends": "tsconfig.base.json" 
   "files": ["testing.ts"]
}

testing.ts

export * from '@my-workspace/lib-a'

Then replace @my-workspace/lib-a to @my-workspace/lib-a/testing in your imports.

This is a tricky way, because no checks will be found for that package, but all the other projects linting checks will work as usually.

forivall commented 1 year ago

there's also the nx-ignore-next-line comment, which will remove the import from the dependency analysis; for example: https://github.com/forivall/nx-repro-local-plugin-issue/commit/341c145f011fa142744fbec86976cd1b86618a01

https://github.com/forivall/nx-repro-local-plugin-issue/blob/341c145f011fa142744fbec86976cd1b86618a01/packages/demo/src/lib/demo.ts#L2-L3

marcus-sa commented 1 year ago

How is this still not supported? https://github.com/nrwl/nx/issues/9645

mxvsh commented 5 months ago

what is the way to disable the check for certain package?

nrwl / nx