Skip to content

Commit

Permalink
feat(@angular/cli): add circular dependency detection
Browse files Browse the repository at this point in the history
Circular dependencies, like `app.module.ts` importing `app.component.ts` which in turn imports `app.module.ts`, now display a warning during builds:

```
kamik@T460p MINGW64 /d/sandbox/master-project (master)
$ ng build
Hash: 3516b252f4e32d6c5bb8
Time: 8693ms
chunk    {0} polyfills.bundle.js, polyfills.bundle.js.map (polyfills) 160 kB {4} [initial] [rendered]
chunk    {1} main.bundle.js, main.bundle.js.map (main) 5.95 kB {3} [initial] [rendered]
chunk    {2} styles.bundle.js, styles.bundle.js.map (styles) 10.5 kB {4} [initial] [rendered]
chunk    {3} vendor.bundle.js, vendor.bundle.js.map (vendor) 1.88 MB [initial] [rendered]
chunk    {4} inline.bundle.js, inline.bundle.js.map (inline) 0 bytes [entry] [rendered]

WARNING in Circular dependency detected:
src\app\app.module.ts -> src\app\app.component.ts -> src\app\app.module.ts

WARNING in Circular dependency detected:
src\app\app.component.ts -> src\app\app.module.ts -> src\app\app.component.ts
```

It is important to detect and eliminate circular dependencies because leaving them in might lead to `Maximum call stack size exceeded` errors, or imports being `undefined` at runtime.

To remove these warnings from your project you can factor out the circular dependency into a separate module.

For instance, if module A imports `foo` from module B, and module B imports `bar` from module A, it is enough to extract `foo` into module C.

You can turn off these warnings by running ng set apps.0.hideCircularDependencyWarnings=true. This will add the "hideCircularDependencyWarnings": true value to your .angular-cli.json and disable the warnings.

Fix angular#6309
Fix angular#6739
  • Loading branch information
filipesilva authored and danielronnkvist committed Jun 28, 2017
1 parent 5f39d1c commit 435917b
Show file tree
Hide file tree
Showing 7 changed files with 37 additions and 0 deletions.
1 change: 1 addition & 0 deletions docs/documentation/angular-cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
- *testTsconfig* (`string`): The name of the TypeScript configuration file for unit tests.
- *prefix* (`string`): The prefix to apply to generated selectors.
- *serviceWorker* (`boolean`): Experimental support for a service worker from @angular/service-worker. Default is `false`.
- *hideCircularDependencyWarnings* (`boolean`): Hide circular dependency warnings on builds. Default is `false`.
- *styles* (`string|array`): Global styles to be included in the build.
- *stylePreprocessorOptions* : Options to pass to style preprocessors.
- *includePaths* (`array`): Paths to include. Paths will be resolved to project root.
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
"dependencies": {
"autoprefixer": "^6.5.3",
"chalk": "^1.1.3",
"circular-dependency-plugin": "^3.0.0",
"common-tags": "^1.3.1",
"css-loader": "^0.28.1",
"cssnano": "^3.10.0",
Expand Down
5 changes: 5 additions & 0 deletions packages/@angular/cli/lib/config/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,11 @@
"type": "boolean",
"default": false
},
"hideCircularDependencyWarnings": {
"description": "Hide circular dependency warnings on builds.",
"type": "boolean",
"default": false
},
"styles": {
"description": "Global styles to be included in the build.",
"type": "array",
Expand Down
7 changes: 7 additions & 0 deletions packages/@angular/cli/models/webpack-configs/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { extraEntryParser, getOutputHashFormat } from './utils';
import { WebpackConfigOptions } from '../webpack-config';

const ProgressPlugin = require('webpack/lib/ProgressPlugin');
const CircularDependencyPlugin = require('circular-dependency-plugin');


/**
Expand Down Expand Up @@ -72,6 +73,12 @@ export function getCommonConfig(wco: WebpackConfigOptions) {
}));
}

if (!appConfig.hideCircularDependencyWarnings) {
extraPlugins.push(new CircularDependencyPlugin({
exclude: /(\\|\/)node_modules(\\|\/)/
}));
}

return {
resolve: {
extensions: ['.ts', '.js'],
Expand Down
1 change: 1 addition & 0 deletions packages/@angular/cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
"@ngtools/webpack": "1.5.0-rc.1",
"autoprefixer": "^6.5.3",
"chalk": "^1.1.3",
"circular-dependency-plugin": "^3.0.0",
"common-tags": "^1.3.1",
"css-loader": "^0.28.1",
"cssnano": "^3.10.0",
Expand Down
4 changes: 4 additions & 0 deletions packages/@angular/cli/tasks/eject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ const ExtractTextPlugin = require('extract-text-webpack-plugin');
const HtmlWebpackPlugin = require('html-webpack-plugin');
const SilentError = require('silent-error');
const licensePlugin = require('license-webpack-plugin');
const CircularDependencyPlugin = require('circular-dependency-plugin');
const Task = require('../ember-cli/lib/models/task');

const ProgressPlugin = require('webpack/lib/ProgressPlugin');
Expand Down Expand Up @@ -189,6 +190,9 @@ class JsonWebpackSerializer {
args = this._extractTextPluginSerialize(plugin);
this.variableImports['extract-text-webpack-plugin'] = 'ExtractTextPlugin';
break;
case CircularDependencyPlugin:
this.variableImports['circular-dependency-plugin'] = 'CircularDependencyPlugin';
break;
case AotPlugin:
args = this._aotPluginSerialize(plugin);
this._addImport('@ngtools/webpack', 'AotPlugin');
Expand Down
18 changes: 18 additions & 0 deletions tests/e2e/tests/misc/circular-dependency.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { prependToFile } from '../../utils/fs';
import { ng } from '../../utils/process';


export default async function () {
await prependToFile('src/app/app.component.ts',
`import { AppModule } from './app.module'; console.log(AppModule);`);
let output = await ng('build');
if (!output.stdout.match(/WARNING in Circular dependency detected/)) {
throw new Error('Expected to have circular dependency warning in output.');
}

await ng('set', 'apps.0.hideCircularDependencyWarnings=true');
output = await ng('build');
if (output.stdout.match(/WARNING in Circular dependency detected/)) {
throw new Error('Expected to not have circular dependency warning in output.');
}
}

0 comments on commit 435917b

Please sign in to comment.