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: Ability to skip an import #188

Open
abierbaum opened this issue Sep 24, 2019 · 10 comments
Open

Feature request: Ability to skip an import #188

abierbaum opened this issue Sep 24, 2019 · 10 comments
Labels

Comments

@abierbaum
Copy link

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';
@sverweij
Copy link
Owner

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.

@abierbaum
Copy link
Author

@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.

@jomi-se
Copy link

jomi-se commented Aug 13, 2020

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
Copy link

jomi-se commented Aug 13, 2020

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 🤦

@AlanOrtega91
Copy link

@abierbaum did you found a workaround?

@abierbaum
Copy link
Author

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

@AlanOrtega91
Copy link

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

@robatwilliams
Copy link

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.

@ben-eb
Copy link

ben-eb commented Jan 24, 2024

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/

@sverweij
Copy link
Owner

sverweij commented Feb 6, 2024

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-violations flag to ignore violations in that baseline and the err-html and markdown reporters with foldouts to show/ hide violations in the baseline.

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

No branches or pull requests

6 participants