bcherny/tslint-no-circular-imports

TSLint plugin to detect and warn about circular imports

tslint-no-circular-imports

TSLint plugin to detect and warn about circular imports

Installation

# Using Yarn:
yarn add --dev tslint-no-circular-imports

# Or, using NPM:
npm install --save-dev tslint-no-circular-imports

Usage

Add the following to your tslint.json:

{
  "extends": ["tslint-no-circular-imports"]
}

Run TSLint:

$ tslint .
Circular import detected: foo.ts -> bar.ts -> foo.ts
Circular import detected: baz.ts -> bar.ts -> baz.ts

Note, if you wish to have circular references reported as a warning, rather than as an error, then add this to your tslint.json:

{
  "rules": {
    "no-circular-imports": { "severity": "warn" }
  }
}

Running the tests

npm test

License

MIT

tslint-no-circular-imports's Issues

Running tslint . is deprecated

> [email protected] lint D:\Code\tslint-no-circular-imports
> tslint .

'.' does not exist. This will be an error in TSLint 6.

Things move so fast these days...

Edit: I started working on a quick PR then realized there's no tslint.json checked in here. Is that intentional?

Crashes if no config option is provided

This was me ☹ #23's code does not check if ruleArguments[0] exists.

Problem about the weird presentation of circular imports

I'm currently curious about the warning texts problem , which I'm not sure whether original feature design or not. The situation code could clone from the commit: https://github.com/chigix/mandoc/tree/8fb20574682c5335ff90eea4761c82d1a4021dfe

after clone and checkout to commit: 8fb20574682c5335ff90eea4761c82d1a4021dfe, run:

npm install
npm run ts-lint

> tslint -c tslint.json --project tsconfig/tsconfig.base.json "src/**/*.ts"

no-unused-variable is deprecated. Since TypeScript 2.9. Please use the built-in compiler checks instead.

ERROR: C:/Users/chigi/Projects/mandoc/src/bin/mandoc.ts[17, 1]: circular import detected: mandoc.ts -> template.config.ts -> template.stream.ts -> template.helper.ts -> mandoc.ts
ERROR: C:/Users/chigi/Projects/mandoc/src/index.ts[1, 1]: circular import detected: index.ts -> template.stream.ts -> template.helper.ts -> index.ts
....

The problem is about the circulation result. Take the first error for instance, the real circulation should be:

mandoc.ts -> template.config.ts -> template.stream.ts -> template.helper.ts -> template.stream.ts

rather than:

mandoc.ts -> template.config.ts -> template.stream.ts -> template.helper.ts -> mandoc.ts

, which made me confused initially because I could not find any import from mandoc.ts in template.helper.ts, as warned.

Although it is not a fatal error, I still want to ask and confirm the original design.

Tests always fail on Windows

After running npm run test or just node test/test.js:

AssertionError [ERR_ASSERTION]: [ { failure:
     'circular import detected: case1.ts -> case1.2.ts -> case1.ts',
    name: 'D:/Code/tslint-no-circular-impor... deepEqual [ { failure:
     'circular import detected: case1.ts -> case1.2.ts -> case1.ts',
    name: 'D:\\Code\\tslint-no-circular-imp...
    at child_process_1.exec (D:\Code\tslint-no-circular-imports\test\test.js:17:12)
    at ChildProcess.exithandler (child_process.js:298:5)
    at ChildProcess.emit (events.js:182:13)
    at maybeClose (internal/child_process.js:961:16)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:248:5)

#13 would fix this nicely. @geanpeixoto if you don't have the time to come back to this, would you like me to open a new PR with merge conflicts resolved and place some sample output in it too?

Naming a file with the same name as an imported library causes false alert

We have a file named history.ts which takes care of initializing the history. It uses the history module from NPM within it, which causes the lint to produce false alerts. The code is like so:

// history.ts

import { createMemoryHistory } from 'history';

This causes an error for dependency chain history -> history. Is there a way to easily fix this issue, other than to rename our file?

Set to warning state?

Not seeing in the docs how to make this simply display as a warning, rather than an error?

Error: Maximum call stack size exceeded

When a project has too many files getAllCycles eventually will hit the call stack size limit:

The 'no-circular-imports' rule threw an error in 'file.tsx':
RangeError: Maximum call stack size exceeded
    at NoCircularImportsWalker.getAllCycles (/Users/user/project/node_modules/tslint-no-circular-imports/noCircularImportsRule.js:195:30)
    at NoCircularImportsWalker.getAllCycles (/Users/user/project/node_modules/tslint-no-circular-imports/noCircularImportsRule.js:193:30)
    at NoCircularImportsWalker.getAllCycles (/Users/user/project/node_modules/tslint-no-circular-imports/noCircularImportsRule.js:193:30)
    at NoCircularImportsWalker.getAllCycles (/Users/user/project/node_modules/tslint-no-circular-imports/noCircularImportsRule.js:193:30)
    at NoCircularImportsWalker.getAllCycles (/Users/user/project/node_modules/tslint-no-circular-imports/noCircularImportsRule.js:193:30)
    at NoCircularImportsWalker.getAllCycles (/Users/user/project/node_modules/tslint-no-circular-imports/noCircularImportsRule.js:193:30)
    at NoCircularImportsWalker.getAllCycles (/Users/user/project/node_modules/tslint-no-circular-imports/noCircularImportsRule.js:193:30)
    at NoCircularImportsWalker.getAllCycles (/Users/user/project/node_modules/tslint-no-circular-imports/noCircularImportsRule.js:193:30)
    at NoCircularImportsWalker.visitSourceFile (/Users/user/project/node_modules/tslint-no-circular-imports/noCircularImportsRule.js:102:34)
    at NoCircularImportsWalker.SyntaxWalker.visitNode (/Users/user/project/node_modules/tslint/lib/language/walker/syntaxWalker.js:482:22)

I've tried to refactor it so it will be asynchronus but it ran out of memory 😞
Is there any way to make it work with large projects?

Recommend using a Webpack- rather than a TS-Lint Plugin if using Webpack

This plugin was exactly what I was looking for, thanks for writing it!

However, it has some pain points:

• It treats type imports the same as non-type imports (which is a deal breaker)
• It is very slow and consumes a lot of memory (I got an out of heap exception)
• It makes live (<5s) ts-lint feedback impossible

If using webpack, I can recommend using circular-dependency-plugin instead.
As it operates on the transpiled modules, it does not consider circular type imports.
It is also way faster.

Warning about type information

Hi,

I really need that package for my project so I am really glad I found it here!

But I have a problem, when I run
$ tslint -c tslint.json 'src/**/*.ts'
I get this error :
Warning: The 'no-circular-imports' rule requires type information.

Here are my versions :

"tslint": "^5.10.0",
"tslint-no-circular-imports": "^0.4.0"

Thanks for the help

The plugin crashes if you export './index'

I was just using the plugin and I had an index file which contained the following line by mistake.

export * from './index'

This code crashes the plugin with the following error:

TypeError: Cannot read property 'getStart' of undefined

I have created a project which demonstrates the bug here: https://github.com/w0rp/no-circular-import-index-error

You can run yarn && node_modules/.bin/tslint -p . on that project to see the error, or you can create a similar file and try it yourself. I think that bug should be fixed, and it should be reported as an error.

Infinite loop after stack overflow fix

After merging #31 (which seemed to be a perfect fix for me) the linting never finishes for some of my projects. I guess that we should add some kind of limiter there.

NPM version differs from repo version

Can anyone tell me, where the 0.7.0 version on npm comes from? It's neither in the repo's package.json nor in its changlog.

Circular Dependency when only type is used

Hello,
I have four classes:

class Starter {
    public getRunner(): AbstractRunner {
        if (something_to_check) {
            return new RunnerA(this);
        } 
        return new RunnerB(this);
    }
}
new Start().getRunner().run();

class AbstractRunner {
   constructor(protected starter: Starter)
    public run(): boolean {
       .. do something...
    }
}

class RunnerA extends AbstractRunner {}

class RunnerB extends AbstractRunner {}

Sadly, this linter causes error:
circular import detected: Starter.ts -> AbstractRunner.ts -> Starter.ts
There is circual dependency by only for typing in AbstractRunner constructor, not used anything else. Could it be skipped? Any idea?

Regards

Update to latest version of tslint?

Any plans to maintain this project?
Running it with current version (5.4.3) a warning is logged about

Warning: Cannot read property 'resolvedFileName' of undefined

I guess https://github.com/bcherny/tslint-no-circular-imports/blob/master/noCircularImportsRule.ts#L34 doesn't work anymore?

Also there are a lot of APIs that are used in the rule being marked as deprecated...

I might be able to support this endeavor.

Best Christian

Ignoring interfaces?

Yo! This is a great plugin - helps me avoid some hard-to-track bugs!

Is there any way we could have an option to ignore interfaces / types? Circular references between interfaces do not cause runtime issues and my project has a few that are hard to get rid of!

Convert to a walk function instead of a RuleWalker?

See discussion in palantir/tslint#2522.

Tracking issue behind changes being split out of #20.

No reports are generated for tslint and Angular 4 project

When I run ng build --scd then I see all circular dependencies reported. However, tslint-no-circular-imports doesn't show any errors while using with tslint in Angular 4 project.

It seems that condition !ts.isSourceFile is not passing. I do not know why, at the moment. The console.log(node.parent) returns SourceFileObject{ here is the content };

However, even, if I skip condition if (!node.parent || !ts.isSourceFile(node.parent) I am getting result: All files pass linting..

Consider to add some changelog (enhancement)

Hi @bcherny,
I'm a happy user of your package!
Well, I don't want to seem intrusive but It would be awesome if you could support some changelog.md to see your fixes and improvements more easily.
Other good option could be GitHub Releases

What do you think?

Warning: The 'no-circular-imports' rule requires type checking

Not sure what to do about this warning... the plugin doesnt appear to be working.

Various problems running within in Visual Studio

I'm using this in Visual Studio with tslint-language-service. There are a number of problems we've had.

• VS 2017 latest seems to come bundled with node v5.4.1, which doesn't appear support all the syntax in the ES2015 build (#6)
• The rules work the first time the linter is run, but any errors remain even after they're fixed. The only way to stop them is to reopen the project (#10 )
• The tests in this repo don't work on Windows (#5)
• The typings for node get dragged in when we install the package, and cause conflicts in our main project. I suspect they should be a dev dependency? (#7 , #8 )

We have fixed these locally, so I'm happy to PR them - do you prefer separate PRs for the separate issues or all together?