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

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

Closed
DerStimmler opened this issue Nov 10, 2022 · 10 comments · Fixed by #592
Closed
Labels
bug Something isn't working outdated scope: core

Comments

@DerStimmler
Copy link
Contributor

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 nrwl/nx#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:

  • OS: Windows
  • nx-dotnet Version 1.16.0
  • nx Version 14.5.1
@DerStimmler DerStimmler added bug Something isn't working needs-triage This issue has yet to be looked over by a core team member labels Nov 10, 2022
@AgentEnder AgentEnder added needs-triage This issue has yet to be looked over by a core team member scope: core and removed needs-triage This issue has yet to be looked over by a core team member labels Nov 10, 2022
@AgentEnder
Copy link
Member

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

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

@AgentEnder
Copy link
Member

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

@DerStimmler
Copy link
Contributor Author

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

@DerStimmler
Copy link
Contributor Author

DerStimmler commented Jan 22, 2023

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

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

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

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

@github-actions
Copy link
Contributor

github-actions bot commented Feb 1, 2023

🎉 This issue has been resolved in version 1.19.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2023

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working outdated scope: core
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants