softarc-consulting / sheriff

Lightweight Modularity for TypeScript Projects
MIT License
183 stars 14 forks source link

Help Needed: sheriff.config Rules Not Enforcing as Expected #117

Open alcaidio opened 3 weeks ago

alcaidio commented 3 weeks ago

I thought this was the best place to post my issue after stackoverflow.

I'm having trouble understanding why I don't get an error when the rules defined in sheriff.config are violated. For example, when placing a component from one domain into another, or placing a feature component in the ui or util folders.

Here is my sheriff.config:

export const config: SheriffConfig = {
  version: 1,
  entryFile: 'src/main.ts',
  tagging: {
    src: {
      app: {
        'domains/<domain>': {
          '<type>/*': ['domain:<domain>', 'type:<type>'],
        },
        'shared/<type>/*': ['domain:shared', 'type:<type>'],
        'openapi/<domain>/*': ['domain:<domain>', 'type:util'],
      },
      environments: ['domain:shared', 'type:util'],
    },
  },
  depRules: {
    root: ['*'],
    'domain:*': [sameTag, 'domain:shared'],
    'type:feature': [
      ({ to }) => to.startsWith('type:ui'),
      'type:data',
      'type:util',
    ],
    'type:ui': ['type:data', 'type:util'],
    'type:data': ['type:util'],
    'type:util': noDependencies,
  },
};

And here is the a partial output of npx sheriff list command:

. (root)
└── src
  ├── app
    ├── domains
      ├── device
        ├── data (domain:device, type:data)
        ├── feature (domain:device, type:feature)
          ├── default-parameters (domain:device, type:feature/default-parameters)
          └── tracker-details (domain:device, type:feature/tracker-details)
        ├── ui (domain:device, type:ui)
        └── util (domain:device, type:util)
      └── gateway
        ├── data (domain:gateway, type:data)
        ├── feature (domain:gateway, type:feature)
          └── gateway-details (domain:gateway, type:feature/gateway-details)
        ├── ui (domain:gateway, type:ui)
        └── util (domain:gateway, type:util)
    ├── openapi
      ├── gateway (domain:gateway, type:util)
        ├── api (domain:gateway/api, type:util)
        └── model (domain:gateway/model, type:util)
    └── shared
      ├── data (domain:shared, type:data)
      ├── feature (domain:shared, type:feature)
      ├── ui (domain:shared, type:ui)
        ├── card (domain:shared, type:ui/card)
      └── util (domain:shared, type:util)
  └── environments (domain:shared, type:util)

Here's the output of npx sheriff verify:

Verification Report :

No issues found. Well done!

Even if i dont respect the config...

I'm not sure how to resolve this issue. Does anyone have any ideas?

rainerhahnekamp commented 3 weeks ago

It would be great to have a working example, but since you've already posted the config, it is enough if you take the file in question and post its path and its import statements.

rainerhahnekamp commented 2 weeks ago

Just noticed that you have all your features in one parent folder feature which seems to be a module on its own?

Shouldn't it be more like?

. (root)
└── src
  ├── app
    ├── domains
      ├── device
        ├── data (domain:device, type:data)
        ├── feature
          ├── default-parameters (domain:device, type:feature)
          └── tracker-details (domain:device, type:feature)

In that case, your config should look like this:

export const config: SheriffConfig = {
  version: 1,
  entryFile: 'src/main.ts',
  tagging: {
    src: {
      app: {
        'domains/<domain>': {
          'feature/<feature>: ['domain:<domain>', 'type:<type>'],
          '<type>': ['domain:<domain>', 'type:<type>'],
        },
        'shared/<type>/*': ['domain:shared', 'type:<type>'],
        'openapi/<domain>/*': ['domain:<domain>', 'type:util'],
      },
      environments: ['domain:shared', 'type:util'],
    },
  },
  depRules: {
    // ...
  },
};
alcaidio commented 2 weeks ago

Just noticed that you have all your features in one parent folder feature which seems to be a module on its own?

Shouldn't it be more like?

. (root)
└── src
  ├── app
    ├── domains
      ├── device
        ├── data (domain:device, type:data)
        ├── feature
          ├── default-parameters (domain:device, type:feature)
          └── tracker-details (domain:device, type:feature)

Yes here a screenshot Screenshot from 2024-08-26 15-28-54

I dont understand your exemple

'feature/<feature>': ['domain:<domain>', 'type:<type>'],
<feature> is not used ?
alcaidio commented 2 weeks ago

It would be great to have a working example, but since you've already posted the config, it is enough if you take the file in question and post its path and its import statements.

For instance here i'm in the domain device, and i can import a component from an other domain (here DDSComponent) Screenshot from 2024-08-26 15-39-11 path : src/app/domains/device/feature/firmwares.page.ts

rainerhahnekamp commented 2 weeks ago

@alcaidio Okay, in which folder is this file located?

alcaidio commented 2 weeks ago

@alcaidio Okay, in which folder is this file located?

path : src/app/domains/device/feature/firmwares.page.ts

rainerhahnekamp commented 2 weeks ago

@alcaidio I encountered a similar issue last week but wasn't sure if it was a bug or misconfiguration. I'll investigate and mark this issue as "potential bug" until further notice.

rainerhahnekamp commented 2 weeks ago

@alcaidio, I encountered a bug which might be related to your reported issue. Fix will come coon.

rainerhahnekamp commented 1 week ago

@alcaidio, can you please try it out with v0.17.0? I can confirm that the new dependency rules algorithm works on a really project and I hope that it was the cause of your issue.

alcaidio commented 1 week ago

@rainerhahnekamp i try with

@softarc/eslint-plugin-sheriff": "^0.17.0",
@softarc/sheriff-core": "^0.17.0",

But nothing change, i always can import component from a domain to another :/

rainerhahnekamp commented 1 week ago

That's quite challenging. I could maybe come up with an explain command in the CLI which shows for every import the module and the applied rule.

Is there a way, I can take a look at your application?

hoangvutruong513 commented 1 week ago

Hello @rainerhahnekamp, @alcaidio.

I think I actually encountered this issue before, where Sheriff was unable to flag a forbidden module import as a Lint error.

I notice that in @alcaidio screenshot, the full path src/app/... is being used to import modules.

As weird as this may sounds:

  1. This error happens when I was importing modules using the full import path, e.g. src/app/path/to/forbidden/module, like @alcaidio is doing. In this case, Sheriff is unable to flag the import as forbidden.

  2. The error is gone, when I switch to using relative path to import modules, e.g. ../../../path/to/forbidden/module. In this case, Sheriff is able to flag the import as forbidden.

  3. The error is also gone, when I use path substitution in tsconfig.json to import modules. Basically in tsconfig.json:

    {
    "compilerOptions": {
    // other configs
    "paths": {
      "@subs/*":  ["src/app/*"]
    }
    }

    Then in your code, use @subs/path/to/forbidden/module. Sheriff would be able to flag that this is a forbidden import.

I am unable to share screenshots due to company policy, but I hope the above info is enough for @rainerhahnekamp to do a bug re-production.

@alcaidio may be you can try switching your import paths to option (2) or (3) above and see if your problem is fixed.

I am using Sheriff version 0.16 by the way.

rainerhahnekamp commented 1 week ago

@hoangvutruong513 thanks for that tip.

It could be that these absolute imports couldn't be properly resolved. In case that happens, Sheriff silently skips them because they should throw a TypeScript compilation error either way.

Thanks, maybe there is connection to #122

I will investigate further