Add external imports whitelist logic to nx-enforce-module-boundaries (i.e. allowedExternalImports option)

[yjaaidi]

• I'd be willing to implement this feature (contributing guide)

Description

nx-enforce-module-boundaries should allow a deny-all + whitelist logic in addition to the blacklist logic.

Motivation

There are use cases where users might need more restrictive rules and only whitelist some external imports instead of explicitly banning them.

Here is one, when implementing something similar hexagonal, onion, or clean architecture, it is safer to use whitelists.
For instance, a library tagged as "type:core" will not be able to import anything using something like this allowedExternalImports: [], while a "platform:backend" & "type:data-access" will be allowed to use libraries that can communicate with databases or remote services: allowedExternalImports: ['mongo', '@google-cloud/*', ...] or something like this.

Suggested Implementation

We could implement this using an allowedExternalImports or whitelistedExternalImports option.

• allowedExternalImports is exclusive with bannedExternalImports. We can't use both on the same rule.
• Nice to have: It would be nice to figure out a way of combining rules. If the project matches multiple rules, then the allowed imports should be merged together

Alternate Implementations

Meanwhile, here is the workaround:

"bannedExternalImports": [
    "((?!(@lib-a/*|@lib-b/something|lib-c)).)+"
 ]

Special thanks @meeroslav for the hint 😉

cf. #6727 (comment)
cc. @mlebarron

[meeroslav]

Thanks @yjaaidi,

After thinking more about this, I think it makes sense. Despite having the possibility to use regex, I think that would be too complex for most folks. We need something that is simple and easy to read, and allowedExternalImports would be exactly that.

Please note that bannedExternalImports has pairing banTransitiveDependencies that toggles the check on transitive dependencies. We could reuse the same for allowedExternalImports to perform checks on nested projects.

[meeroslav]

Feel free to implement this feature and ping me if you need any help.

[yjaaidi]

Cool! I'll submit a PR as soon as I can... expect some questions 😊

First one is about your remark:

Please note that bannedExternalImports has pairing banTransitiveDependencies that toggles the check on transitive dependencies. We could reuse the same for allowedExternalImports to perform checks on nested projects.

Is this related to checkNestedExternalImports?
I don't really get why we would go through nested projects as we are just whitelisting direct external imports. I think that I am missing something.

[meeroslav]

I don't really get why we would go through nested projects as we are just whitelisting direct external imports. I think that I am missing something.

True, it doesn't matter in the opposite direction. Sorry for the confusion

[meeroslav]

Hey @yjaaidi, do you plan to work on this?

[yjaaidi]

Hey @meeroslav! Yes ! Sorry that I couldn't take care of it before. I'll take care of by next week.

[yjaaidi]

Hey @meeroslav! I finally managed to take some time and send a PR #13891 🎉

I was about to send a distinct PR for the docs as I thought that this would require a major rewrite of the "Ban External Imports" recipe but I managed to just add a chapter at the end with some examples.

Let me know what you think of this and if you want me to squash everything.

Good job for the allSourceTags btw 😉

[meeroslav]

Implemented with #13891

[github-actions]

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.