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

Disable Circular Dependency Check #13842

Open
dcrdev opened this issue Dec 15, 2022 · 6 comments
Open

Disable Circular Dependency Check #13842

dcrdev opened this issue Dec 15, 2022 · 6 comments
Assignees
Labels
scope: linter Issues related to Eslint support in Nx type: feature

Comments

@dcrdev
Copy link

dcrdev commented Dec 15, 2022

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:

  1. Use explicit dependsOn directives with the !package-name syntax
    This does not work at all in my situation

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

@FrozenPandaz FrozenPandaz added the scope: core core nx functionality label Dec 20, 2022
@stevenxu-db
Copy link

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
Copy link

g3tr1ght commented Dec 22, 2022

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
Copy link

angelfraga commented Feb 24, 2023

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
Copy link

angelfraga commented Feb 27, 2023

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
Copy link
Contributor

forivall commented Mar 7, 2023

there's also the nx-ignore-next-line comment, which will remove the import from the dependency analysis; for example: forivall/nx-repro-local-plugin-issue@341c145

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

@marcus-sa
Copy link

How is this still not supported?
#9645

@FrozenPandaz FrozenPandaz added scope: linter Issues related to Eslint support in Nx and removed scope: core core nx functionality labels Apr 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope: linter Issues related to Eslint support in Nx type: feature
Projects
None yet
Development

No branches or pull requests

8 participants