modules-graph-assert
modules-graph-assert copied to clipboard
Add whitelist to enable gradual refactoring
Defining rules for a green field project is a luxury that we don't have most of the times. In reality we are 'adding' rules to existing projects and there is a high probabality that one or more modules are violating that rule. Therefore, having a temporary whitelist can help us introduce new restricting rules, so no additional violating dependency is added to project, mean while team has time to refactor the project and remove the current dependency violations.
p.s. I want to achieve this without defining alias.
Hi @sherviiin,
Thank you very much for the contribution, I appreciate that. 👍
I would though not accept the PR from the following reasons -
- There are already 2 different ways how to achieve the same thing. Use the moduleNameAlias (I read you won't want to use it - why? :) or using the
allowedrules with explicitly listing the exceptions. - I prefer not to have another way of doing things as thing might get more confusing like that.
- I prefer the tasks logic to be completely independent to maintain simplicity - meaning the
RestrictedDependenciesAssertwill now be dependent on thewhitelist. This brings a question if there should be dependency onallowed, because having onlywhitelistproviding exception and notallowedwould be confusing. Answer to that question can be seen differently by different people.
What is the use case you have in mind for your change where the existing rules won't work please?
Hi @jraska
Thank you for your reply.
The reason I did not go with allowed rules and listing the exceptions is "too many rules, too many exceptions"!
In an existing project both of those lists can be enormous, while introducing a restriction and adding whitelist for it can be done for 1 single module.
About possible confusions between whitelist & allowed (which I totally agree with), I can make it more transparent for user. I will probably do that today/tmrw.
About possible confusions between whitelist & allowed (which I totally agree with), I can make it more transparent for user. I will probably do that today/tmrw.
Hey, I assume finding the time for this is tricky :)
The problem still stays though that we should possibly consider how the code would look like with using the allowed/restricted. We surely should not add new whitelist as explained above.
Then comes a discussion if allowed/restricted should be even dependent on each other - I believe that not with reasons explained here and some examples below on same issue.
The other point is that this would be also breaking change in assertions behaviour and would require version 3.0 so even if we would make it work the change would go to shelf for version 3.0 🤷
Could you please share your example of concrete rules how they would look now and how they would look with allowed whitelisting the restricted rules? We can discuss them here.
Hey!
I ended up adding exceptions to both allowed/restricted, so it's possible to add exceptions to any of those. This helps with gradually refactor code and to adhere to newly created rule.
And with that, this PR is no longer valid. Please check my fork to see if you want include that in original repository. If so I can create PR from that.
Thanks.