[entry-point] target option has captured values from the target dependency when templating ${from}.PROP

[DeyLak]

This plugin is so awesome, I finally can force architectural decisions not only during the code review. Thank you for creating and sharing it!

I think, I've encountered a problem similar to #212

Describe the bug
The target option in the boundaries/entry-point configuration can't be used with from templating, cause instead of captured values for from the values are from target.

To Reproduce
I have a layered project structure like this:

src/
├── entities/
├── widgets/
│   ├── foo/
│   │   ├── components/
│   │   ├── hooks/
│   │   ├── helpers/
│   │   ├── ...
│   │   └── index.ts
│   ├── bar/
│   │   ├── components/
│   │   ├── hooks/
│   │   ├── helpers/
│   │   ├── ...
│   │   └── index.ts
├── pages/

Each main layer I've configured as a different type in the boundaries/elements. Also, I'm capturing names of the entities, widgets etc. like foo and bar as slice and nested content like components, hooks etc. as segment.

My goal is to configure boundaries/entry-point to allow importing anything inside the same slice, but not between different slices. Different slices should only use publiс module api defined in index.ts for importing each other.

I'm using the configuration like this:

'boundaries/entry-point': [
  2,
  {
    default: 'disallow',
    rules: [
      {
        target: ['entities', 'widgets'],
        allow: '**',
      },
      {
        target: [
          // Any slice, except the same as target
          ['entities', { slice: '!(${from.slice})' }],
          ['widgets', { slice: '!(${from.slice})' }],
        ],
        // Any file, except index.ts
        disallow: '!(index.ts)',
      },
    ],
  },
],

Other similar configurations with variations on what is defined as the target pattern and what to allow or disallow for it are possible and I tried them, but they all have the same problem, cause they all need to use templating with ${from}.

Expected behavior

This configuration should find errors in imports like:
widgets/foo/components/FooComponent/index.tsx

// This should be an error, only entities/baz is allowed for importing
import { entityHelper } from `entities/baz/helpers.ts`
...

Additional context
I've looked into the code and found out, that when elementRulesAllowEntryPoint is calling elementRulesAllowDependency, it uses getElementRules and passing the dependency there as elementInfo arg.
After that, option.rules are filtered by using ruleMatch. But both targetElement and fromElement arguments for the ruleMatch call are assigned to elementInfo.
So inside the matching both from and target captured values are assigned the same value coming from the dependency, not from element

I've managed to achieve the desired behaviour by changing the getElementRules signature to (elementInfo, options, mainKey, targetElement) and additionally passing element to the function like this:

getElementRules(
    elementToGetRulesFrom(element, dependency, mainKey),
    options,
    mainKey,
    element,
  )

After that, I've changed the filter by passing the targetElement instead of the elementInfo inside the ruleMatch:

return ruleMatch(rule[key], elementInfo, isMatchElementType, targetElement).result;

Here is the full patch I've used:

diff --git a/node_modules/eslint-plugin-boundaries/src/helpers/rules.js b/node_modules/eslint-plugin-boundaries/src/helpers/rules.js
index 33614b3..915d898 100644
--- a/node_modules/eslint-plugin-boundaries/src/helpers/rules.js
+++ b/node_modules/eslint-plugin-boundaries/src/helpers/rules.js
@@ -137,7 +137,7 @@ function isMatchElementType(
   return isMatchElementKey(elementInfo, matcher, options, "type", elementsToCompareCapturedValues);
 }
 
-function getElementRules(elementInfo, options, mainKey) {
+function getElementRules(elementInfo, options, mainKey, targetElement) {
   if (!options.rules) {
     return [];
   }
@@ -150,7 +150,7 @@ function getElementRules(elementInfo, options, mainKey) {
       };
     })
     .filter((rule) => {
-      return ruleMatch(rule[key], elementInfo, isMatchElementType, elementInfo).result;
+      return ruleMatch(rule[key], elementInfo, isMatchElementType, targetElement).result;
     });
 }
 
@@ -176,6 +176,7 @@ function elementRulesAllowDependency({
     elementToGetRulesFrom(element, dependency, mainKey),
     options,
     mainKey,
+    element,
   ).reduce(
     (allowed, rule) => {
       if (rule.disallow) {

I'm not sure about the motivation behind passing the same elementInfo inside the ruleMatch function, so I've decided to open an issue and discuss it first. I can make a PR with this fix, if it wasn't intentional.

[javierbrea]

Hi @DeyLak , thanks for such detailed information in the issue!
What you says makes sense for me. If you open a PR it would be great. Adding some unit tests would be required, so we can confirm that the new behavior work as expected, and also if this breaks some current behavior.

[DeyLak]

@javierbrea here it is: #343

[javierbrea]

Thank you very much for your contribution @DeyLak ! Just released at https://github.com/javierbrea/eslint-plugin-boundaries/releases/tag/v5.0.1 😃