From 1f3738b19f8e4710ef39c9046f70a5fd9b5d9284 Mon Sep 17 00:00:00 2001 From: Filipe Silva Date: Tue, 27 Jun 2017 10:37:27 +0100 Subject: [PATCH] feat(@angular/cli): add circular dependency detection 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 #6309 Fix #6739 --- docs/documentation/angular-cli.md | 1 + package.json | 1 + packages/@angular/cli/lib/config/schema.json | 5 +++++ .../cli/models/webpack-configs/common.ts | 7 +++++++ packages/@angular/cli/package.json | 1 + packages/@angular/cli/tasks/eject.ts | 4 ++++ tests/e2e/tests/misc/circular-dependency.ts | 18 ++++++++++++++++++ 7 files changed, 37 insertions(+) create mode 100644 tests/e2e/tests/misc/circular-dependency.ts diff --git a/docs/documentation/angular-cli.md b/docs/documentation/angular-cli.md index 4adad010f34f..cd8adcfe5a56 100644 --- a/docs/documentation/angular-cli.md +++ b/docs/documentation/angular-cli.md @@ -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. diff --git a/package.json b/package.json index e9300e6fc338..6ca9756f0514 100644 --- a/package.json +++ b/package.json @@ -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", diff --git a/packages/@angular/cli/lib/config/schema.json b/packages/@angular/cli/lib/config/schema.json index 44fc33d7a8d3..a5a956a93e00 100644 --- a/packages/@angular/cli/lib/config/schema.json +++ b/packages/@angular/cli/lib/config/schema.json @@ -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", diff --git a/packages/@angular/cli/models/webpack-configs/common.ts b/packages/@angular/cli/models/webpack-configs/common.ts index a2a2707c8b12..0d54cd52361a 100644 --- a/packages/@angular/cli/models/webpack-configs/common.ts +++ b/packages/@angular/cli/models/webpack-configs/common.ts @@ -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'); /** @@ -72,6 +73,12 @@ export function getCommonConfig(wco: WebpackConfigOptions) { })); } + if (!appConfig.hideCircularDependencyWarnings) { + extraPlugins.push(new CircularDependencyPlugin({ + exclude: /(\\|\/)node_modules(\\|\/)/ + })); + } + return { resolve: { extensions: ['.ts', '.js'], diff --git a/packages/@angular/cli/package.json b/packages/@angular/cli/package.json index 7be4d49f3ea7..d9d1e22448cf 100644 --- a/packages/@angular/cli/package.json +++ b/packages/@angular/cli/package.json @@ -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", diff --git a/packages/@angular/cli/tasks/eject.ts b/packages/@angular/cli/tasks/eject.ts index 331b17b8a957..35598b270f21 100644 --- a/packages/@angular/cli/tasks/eject.ts +++ b/packages/@angular/cli/tasks/eject.ts @@ -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'); @@ -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'); diff --git a/tests/e2e/tests/misc/circular-dependency.ts b/tests/e2e/tests/misc/circular-dependency.ts new file mode 100644 index 000000000000..5b9a0363f5b2 --- /dev/null +++ b/tests/e2e/tests/misc/circular-dependency.ts @@ -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.'); + } +}