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

Feature request: turn off a rule using a template path in config #32

Open
noveogroup-amorgunov opened this issue Jun 11, 2024 · 7 comments

Comments

@noveogroup-amorgunov
Copy link

Hey. I have a feature request: turn off a rule for specific folders/files by path. For example, we have some mocks (which located at an entity layer) and we deliberately don't add them to the public API to don't increase the bundle size.

 ✘ Forbidden sidestep of public API when importing "shared/lib/server/index.ts" from "widgets/ProductDetails/api/__mocks__/widgetProductDetailsHandlers.ts".  // no-public-api-sidestep

 ✘ Forbidden sidestep of public API when importing "entities/product/api/__mocks__/mockProductDtoByIds.ts" from "widgets/BaseProductList/ui/BaseProductList.stories.tsx".  // no-public-api-sidestep

It could look like this:

export default defineConfig({
  rules: {
    'no-public-api-sidestep': 'on',
  },
  overrides: [
    {
      // Maybe we should specify two types of groups ("when importing" files and "from" files) 
      // Allow import any file from `@/shared/lib/server/**/*.ts` from `**/__mocks__/**/*.ts`
      patternFrom: ['**/__mocks__/**/*.ts']
      patternTo: ['@/shared/lib/server/**/*.ts']
      rules: {
        'no-public-api-sidestep': 'off'
      }
    },
    {
      patternFrom: ['*.stories.tsx']
      patternTo: ['**/__mocks__/**/*.ts']
      rules: {
        'no-public-api-sidestep': 'off'
      }
    }
  ]
})
@illright
Copy link
Contributor

illright commented Jun 11, 2024

How valuable is it to hide certain folders on a rule-by-rule basis? I'm asking because we can achieve close enough results by having project-wide globs for ignoring certain folders for all rules, and that would produce a much simpler API. What do you think?

In this scenario we could configure the mock files to be excluded from all rules, and that would do the trick

@noveogroup-amorgunov
Copy link
Author

@illright I think it's good idea) It seems possible to take the approach from git (.gitignore) or eslint (.eslintignore) with separate ignore file (.steigerignore)

Initial example:

# .steigerignore

@/shared/lib/server
**/__mocks__
*.stories.tsx

@noveogroup-amorgunov
Copy link
Author

@illright what do you think about inline file comments for ignore same rules for slices? For example, I have a lot of errors with "insignificant-slice" like "Slice "widgets/XXX" has only one reference on layer "pages". Consider merging them". But widgets are not only for reuse. Widgets are used for self-contained logic and could exist in one instance.

So for this case It would be nice to ignore rule in public API of slice.

Снимок экрана 2024-06-23 в 21 50 00

@illright
Copy link
Contributor

The point about widgets being not only for reuse is a valid one, but rather than make comments, I would rather tweak the rule. Initially, this rule was planned to also measure lines of code, and if there are enough, not to complain. This functionality is not yet implemented though. Could you count the LoC in your project's widgets and post here? Then we'd be able to estimate what is actually an insignificant slice

@illright
Copy link
Contributor

As for control comments like in ESLint — I'm not a big fan of this idea. The reason is that Steiger doesn't work with lines in files, it works with entire files, and usually several. Therefore, there's no single place where the comment should be placed, so it will be confusing to disable rules like that. I'd much rather offer config options to disable certain rules by glob

@noveogroup-amorgunov
Copy link
Author

Initially, this rule was planned to also measure lines of code, and if there are enough, not to complain.

I think the LoC of a widget's shouldn't affect whether it should be a widget or not. We have huge widgets with 10k LoC, and there are very small ones (sliders with products), which are also widgets

@illright
Copy link
Contributor

illright commented Jul 1, 2024

@noveogroup-amorgunov I agree that LoC is generally a trash metric, but it's the first thing that comes to mind. What we need is a good heuristic that would dissuade people from creating slices for everything and then getting lost in 50 slices.

I'm curious about why you have widgets that contain little code. Why did your team decide to make them widgets, instead of keeping them in the page?

Also if you have ideas for a better heuristic, I'm of course glad to hear it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants