nx-dotnet / nx-dotnet

A Nx plugin adding support for .NET 5+ (or .net core) projects, featuring full project graph and generator support.
https://www.nx-dotnet.com/
MIT License
260 stars 62 forks source link

[BUG] CheckModuleBoundaries fails when using 'notDependOnLibsWithTags' configuration #564

Closed DerStimmler closed 1 year ago

DerStimmler commented 1 year ago

Describe the bug Currently it seems that the notDependOnLibsWithTags configuration for the enforce-module-boundaries eslint rule doesn't work with nx-dotnet.

This feature was added in this PR https://github.com/nrwl/nx/pull/8633.

To Reproduce Add the notDependOnLibsWithTags to the enforce-module-boundaries eslint rules. Add the corresponding tags to the project.json of your dotnet project.

"rules": {
        "@nrwl/nx/enforce-module-boundaries": [
          "warn",
          {
            "enforceBuildableLibDependency": true,
            "allowCircularSelfDependency": false,
            "allow": [],
            "depConstraints": [
              {
                "sourceTag": "scope:shared",
                "notDependOnLibsWithTags": ["*"]
              }
           }
          ]
}

Expected behavior The boundaries should be checked like usual with the onlyDependOnLibsWithTags configuration. Just inverted.

Screenshots

 D:\dev\node_modules\@nx-dotnet\core\src\tasks\check-module-boundaries.js:19
          const relevantConstraints = configuredConstraints.filter((x) => tags.includes(x.sourceTag) && !x.onlyDependOnLibsWithTags.includes('*'));
                                                                                                                                    ^

  TypeError: Cannot read properties of undefined (reading 'includes')
      at D:\dev\node_modules\@nx-dotnet\core\src\tasks\check-module-boundaries.js:19:131
      at D:\dev\node_modules\@nx-dotnet\core\src\tasks\check-module-boundaries.js:19:131
      at Array.filter (<anonymous>)
      at Array.filter (<anonymous>)
      at D:\dev\node_modules\@nx-dotnet\core\src\tasks\check-module-boundaries.js:19:59
      at D:\dev\node_modules\@nx-dotnet\core\src\tasks\check-module-boundaries.js:19:59
      at Generator.next (<anonymous>)
      at Generator.next (<anonymous>)
      at fulfilled (D:\dev\node_modules\@nx-dotnet\core\node_modules\tslib\tslib.js:112:62)
      at fulfilled (D:\dev\node_modules\@nx-dotnet\core\node_modules\tslib\tslib.js:112:62)

Environment:

AgentEnder commented 1 year ago

Hey, yeah this plugin's implementation was written before it was added, and the implementation here hasn't been updated.

@DerStimmler do you want to submit a PR to fix this?

DerStimmler commented 1 year ago

@AgentEnder Sure. I'll have a look at it as soon as I find time.

AgentEnder commented 1 year ago

@DerStimmler do you still want this issue? I may be able to pick it up soon if not.

DerStimmler commented 1 year ago

I already worked on it, but struggled with proper testing. Will give it another shot this week.

DerStimmler commented 1 year ago

@AgentEnder I still have problems to test the whole check-module-boundaries feature, because it searches for the .csproj files to read the dependencies of a project. So it's a bit tricky to mock. If you have any idea how we could implement this I'll add some tests to not only test the notDependOnLibsWithTags configuration, but also onlyDependOnLibsWithTags.

I think the general implementation to fix this issue should be done.

Could you please have a quick look over the PR? Because I'm not sure wether it's working correctly without the tests.

AgentEnder commented 1 year ago

You can mock out fast glob in tests, that's probably the best way. I'll lookover the PR, and may push up an example test

AgentEnder commented 1 year ago

Actually, here's an example if you want to mock that out: https://github.com/nrwl/nx/blob/master/packages/nx/src/config/workspaces.spec.ts

DerStimmler commented 1 year ago

Thanks for the example. I added some tests for onlyDependOnLibsWithTags and notDependOnLibsWithTags.

github-actions[bot] commented 1 year ago

:tada: This issue has been resolved in version 1.19.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

github-actions[bot] commented 1 year ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.