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

nx-enforce-module-boundaries restrict importing react in libs #6727

Closed
alexandrzavalii opened this issue Aug 14, 2021 · 5 comments · Fixed by #7378
Closed

nx-enforce-module-boundaries restrict importing react in libs #6727

alexandrzavalii opened this issue Aug 14, 2021 · 5 comments · Fixed by #7378
Assignees
Labels
outdated scope: linter Issues related to Eslint support in Nx type: feature

Comments

@alexandrzavalii
Copy link

alexandrzavalii commented Aug 14, 2021

Description

I have some libraries that are dependent on React and other libraries that are just utils.
I would like to restrict importing of React in libraries that are just utils.

I am not exactly sure how this would look, but something like this would be handy

        "depConstraints": [
          {
            "sourceTag": "scope:lib",
            "onlyDependOnLibsWithTags": ["scope:shared"],
            "notAllowedImports": ["react"]
          }
        ]
@AgentEnder
Copy link
Member

Hey! This is an interesting suggestion 👍.

In the meantime, you can always add react to the no-restricted-imports eslint rule in each library's eslintrc file (https://eslint.org/docs/rules/no-restricted-imports)

@leosvelperez leosvelperez added the scope: linter Issues related to Eslint support in Nx label Aug 27, 2021
@meeroslav meeroslav self-assigned this Sep 20, 2021
@mlebarron
Copy link

is negating this possible? as in:

{
    "sourceTag": "type:util",
    "onlyDependOnLibsWithTags": ["type:util"],
    "bannedExternalImports": ["external-lib"]
},

and then allowing it

{
    "sourceTag": "scope:authentication",
    "onlyDependOnLibsWithTags": ["scope:shared", "scope:authentication"],
    "allowedExternalImports": ["external-lib"]
},

Essentially... I don't want any util libs to use this external lib, but I do have a specific instance where it needs to be allowed, and it's anything in scope:authentication including a type:util lib.

@meeroslav
Copy link
Contributor

@mlebarron the short answer is - No, unfortunately not.

The way dependency constraints work is that they are combined using AND. This means that your example would be translated to:

  • Project of type util cannot import external-lib AND
  • Project of scope authentication can import external-lib

Although scope would allow it, the type util would ban it, resulting in banned import.
Cases like these can be usually resolved just by stepping back and rethinking your tags. We often start with dep-graph diagram and "draw" the boundaries before we type them in the code.

If one of your type:util libs behave differently (allows external-lib) then maybe it should have a different tag.

@mlebarron
Copy link

@meeroslav Yeah that's why I was asking, just wanted to confirm. We happen to have a case where we have a legacy library that we don't want to allow basically anything in the new codebase to use except where absolutely necessary, until it's phased out. We actually discussed redoing the tag structure but figured it wasn't worth it just to accommodate one lib. We're using no-restricted-imports at the moment, which works for our needs.

I didn't give the greatest sample, so it made less sense than if I actually put the library names and full setup :)

@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated scope: linter Issues related to Eslint support in Nx type: feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants