dependency-cruiser icon indicating copy to clipboard operation
dependency-cruiser copied to clipboard

Feature request: Ability to skip an import

Open abierbaum opened this issue 5 years ago • 10 comments

Context

I would like to use dependency-cruiser on our codebase as part of the auto build to validate correct imports. Unfortunately we have many places in the code base that are causing false errors or maybe more specifically, errors we simply want to ignore because they are ok.

Similar to linting tools I would like a way to tell dependency-cruiser to skip the next import line.

I have searched the documentation but can't find a way to do this.

Possible Solution

// dependency-cruiser:ignore-next-line
import {X} from 'bad/package';

abierbaum avatar Sep 24 '19 19:09 abierbaum

hi @abierbaum thanks for this feature request. Before diving into a solution I'd like to understand a bit more about the problem you want to solve.

Let's say you don't want "bad/package" to be used anywhere. Does any of the following describe your needs?

  • "bad/package" currently is used by everything and its mother in the path/to/ignore folder, while it really shouldn't.However, there's other matters that need addressing first. So for now you want to prevent this from flagging errors.
  • There's a specific use case in path/to/exception (and in path/to/specific/file.ts) that warrants the use of bad/package.

If that's the case, you can put the paths (/ file names/ regexp) in a pathNot in the from part of a rule, separated by | to ignore them (this works because path and pathNot are regular expressions).

module.exports = {
  forbidden: [
    {
      name: "not-to-bad-package",
      from: {
        pathNot: "path/to/ignore|path/to/exception|path/to/specific/file\\.ts"
      },
      to: {
        path: "bad/package"
      }
    }
  ]
}

If this doesn't work for your situation - let me know.

sverweij avatar Sep 27 '19 17:09 sverweij

@sverweij In most cases I could probably modify the rules with the path of some files to ignore it for. The issue is that with a code base of thousands of files this can start to build up. So for example I may have 20-30 places I want to ignore a flagged error. I could go modify the rule configurations to add all these files to the rules, but then the rule definition becomes very difficult to maintain and unclear of it's purpose.

I see this as similar to how code linters allow a way to disable a specific linting rule for a line or block of code.

abierbaum avatar Sep 27 '19 19:09 abierbaum

Hello again @sverweij 👋 Hope you're doing fine in these times

I just came across a similar use-case. I wanted to apply a rule similar to the no-inter-ubc described in https://github.com/sverweij/dependency-cruiser/blob/develop/doc/rules-reference.md

The issue is that there are currently ~200 of places where this rule is broken.

Given that it would take a while to fix all of those places, I was trying to figure out a way of activating the rule but ignoring these existing errors. That way, we prevent getting any new code that break this rule in the codebase but we don't have to fix everything upfront.

I thought about including the guilty files in the pathNot as you suggest, but the downside of that approach is that doing that disables this rule for the whole file instead of only silencing the error for that specific import.

The possible solution described by @abierbaum is the one that would follow a somewhat conventional approach. Somewhat like conditionally triggering the same thing as the doNotFollow behaviour if the comment in the line above contains the disabling statement.

Otherwise, we could have a file containing "ignored errors" that would be removed from the final report.

What do you think?

jomi-se avatar Aug 13 '20 16:08 jomi-se

What I'm trying to do for now is write a separate .js file that exports an array of all the names of the files that break this rule. That way I can require it in the dependency cruiser config file and add it to the rule's pathNot property.

However, I'm still getting the errors for some reason 🤔

EDIT: I just realized my issue. I was adding the files generating the errors to the pathNot, but for it to work I would have to add the files being imported/required 🤦

jomi-se avatar Aug 13 '20 16:08 jomi-se

@abierbaum did you found a workaround?

AlanOrtega91 avatar Aug 19 '20 21:08 AlanOrtega91

@AlanOrtega91 No, I never found a workaround. I just limited the rules I used.

abierbaum avatar Aug 19 '20 21:08 abierbaum

@abierbaum sad to hear that :/ guess I'll do the same. Thanks for the quick response!

AlanOrtega91 avatar Aug 19 '20 21:08 AlanOrtega91

Some documentation (and possibly new features) to help introduce dependency-cruiser to an existing codebase would definitely be useful and appreciated. While today most projects start do get started with linting in place for example, I think they would only look for architecture/dependency enforcement once they reach a certain size (code, team) and realise they need it. At that point, it's likely impractical to fix everything from the past immediately.

robatwilliams avatar Apr 10 '21 14:04 robatwilliams

I wonder if writing a Betterer plugin for dependency-cruiser would help with incremental adoption (similar to how their eslint plugin works); https://phenomnomnominal.github.io/betterer/

ben-eb avatar Jan 24 '24 12:01 ben-eb

Hi @ben-eb a plugin for betterer indeed might make sense.

Might be good to know dependency-cruiser already has built-in support for incremental adoption with depcruise-baseline - which creates a baseline, the --ignore-known-ignore-known-violations flag to ignore violations in that baseline and the err-html and markdown reporters with foldouts to show/ hide violations in the baseline.

sverweij avatar Feb 06 '24 19:02 sverweij