From d9e6f7502aab8cdb1cba1c4f5a770a479396449c Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Mon, 31 Jul 2017 21:13:33 +0300 Subject: [PATCH] build: add tslint rule to verify rollup globals config (#5963) Adds a custom tslint rule that ensures that we've added all of the necessary external modules to the Rollup config. This is helpful, because forgetting to add a module will log a warning, but it won't break the build necessarily, which is why we occasionally need PRs like #5930. --- tools/package-tools/rollup-globals.json | 71 +++++++++++++++++++ tools/package-tools/rollup-helpers.ts | 71 +------------------ .../tslint-rules/missingRollupGlobalsRule.js | 43 +++++++++++ tslint.json | 5 ++ 4 files changed, 120 insertions(+), 70 deletions(-) create mode 100644 tools/package-tools/rollup-globals.json create mode 100644 tools/tslint-rules/missingRollupGlobalsRule.js diff --git a/tools/package-tools/rollup-globals.json b/tools/package-tools/rollup-globals.json new file mode 100644 index 00000000000..9862d2f2d69 --- /dev/null +++ b/tools/package-tools/rollup-globals.json @@ -0,0 +1,71 @@ +{ + "tslib": "tslib", + + "@angular/animations": "ng.animations", + "@angular/core": "ng.core", + "@angular/common": "ng.common", + "@angular/forms": "ng.forms", + "@angular/http": "ng.http", + "@angular/router": "ng.router", + "@angular/platform-browser": "ng.platformBrowser", + "@angular/platform-server": "ng.platformServer", + "@angular/platform-browser-dynamic": "ng.platformBrowserDynamic", + "@angular/platform-browser/animations": "ng.platformBrowser.animations", + "@angular/core/testing": "ng.core.testing", + "@angular/common/testing": "ng.common.testing", + "@angular/http/testing": "ng.http.testing", + + "@angular/material": "ng.material", + "@angular/cdk/a11y": "ng.cdk.a11y", + "@angular/cdk/bidi": "ng.cdk.bidi", + "@angular/cdk/coercion": "ng.cdk.coercion", + "@angular/cdk/keyboard": "ng.cdk.keyboard", + "@angular/cdk/observe-content": "ng.cdk.observeContent", + "@angular/cdk/platform": "ng.cdk.platform", + "@angular/cdk/portal": "ng.cdk.portal", + "@angular/cdk/rxjs": "ng.cdk.rxjs", + "@angular/cdk/table": "ng.cdk.table", + "@angular/cdk": "ng.cdk", + "@angular/cdk/testing": "ng.cdk.testing", + + "@angular/material-examples": "ng.materialExamples", + + "rxjs/BehaviorSubject": "Rx", + "rxjs/Observable": "Rx", + "rxjs/Subject": "Rx", + "rxjs/Subscription": "Rx", + "rxjs/Observer": "Rx", + "rxjs/Scheduler": "Rx", + "rxjs/observable/combineLatest": "Rx.Observable", + "rxjs/observable/forkJoin": "Rx.Observable", + "rxjs/observable/fromEvent": "Rx.Observable", + "rxjs/observable/merge": "Rx.Observable", + "rxjs/observable/of": "Rx.Observable", + "rxjs/observable/throw": "Rx.Observable", + "rxjs/operator/auditTime": "Rx.Observable.prototype", + "rxjs/operator/catch": "Rx.Observable.prototype", + "rxjs/operator/debounceTime": "Rx.Observable.prototype", + "rxjs/operator/do": "Rx.Observable.prototype", + "rxjs/operator/filter": "Rx.Observable.prototype", + "rxjs/operator/finally": "Rx.Observable.prototype", + "rxjs/operator/first": "Rx.Observable.prototype", + "rxjs/operator/let": "Rx.Observable.prototype", + "rxjs/operator/map": "Rx.Observable.prototype", + "rxjs/operator/share": "Rx.Observable.prototype", + "rxjs/operator/startWith": "Rx.Observable.prototype", + "rxjs/operator/switchMap": "Rx.Observable.prototype", + "rxjs/operator/takeUntil": "Rx.Observable.prototype", + "rxjs/operator/toPromise": "Rx.Observable.prototype", + + "rxjs/add/observable/merge": "Rx.Observable", + "rxjs/add/observable/fromEvent": "Rx.Observable", + "rxjs/add/observable/of": "Rx.Observable", + "rxjs/add/observable/interval": "Rx.Observable", + "rxjs/add/operator/startWith": "Rx.Observable.prototype", + "rxjs/add/operator/map": "Rx.Observable.prototype", + "rxjs/add/operator/debounceTime": "Rx.Observable.prototype", + "rxjs/add/operator/distinctUntilChanged": "Rx.Observable.prototype", + "rxjs/add/operator/first": "Rx.Observable.prototype", + "rxjs/add/operator/catch": "Rx.Observable.prototype", + "rxjs/add/operator/switchMap": "Rx.Observable.prototype" +} diff --git a/tools/package-tools/rollup-helpers.ts b/tools/package-tools/rollup-helpers.ts index 5db30a14ff0..c4940317741 100644 --- a/tools/package-tools/rollup-helpers.ts +++ b/tools/package-tools/rollup-helpers.ts @@ -4,76 +4,7 @@ import {rollupRemoveLicensesPlugin} from './rollup-remove-licenses'; // There are no type definitions available for these imports. const rollup = require('rollup'); const rollupNodeResolutionPlugin = require('rollup-plugin-node-resolve'); - -const ROLLUP_GLOBALS = { - // Import tslib rather than having TypeScript output its helpers multiple times. - // See https://github.com/Microsoft/tslib - 'tslib': 'tslib', - - // Angular dependencies - '@angular/animations': 'ng.animations', - '@angular/core': 'ng.core', - '@angular/common': 'ng.common', - '@angular/forms': 'ng.forms', - '@angular/http': 'ng.http', - '@angular/platform-browser': 'ng.platformBrowser', - '@angular/platform-browser-dynamic': 'ng.platformBrowserDynamic', - '@angular/platform-browser/animations': 'ng.platformBrowser.animations', - - // Local Angular packages inside of Material. - '@angular/material': 'ng.material', - - // TODO(jelbourn): don't hard-code these - '@angular/cdk/a11y': 'ng.cdk.a11y', - '@angular/cdk/bidi': 'ng.cdk.bidi', - '@angular/cdk/coercion': 'ng.cdk.coercion', - '@angular/cdk/keyboard': 'ng.cdk.keyboard', - '@angular/cdk/observe-content': 'ng.cdk.observeContent', - '@angular/cdk/platform': 'ng.cdk.platform', - '@angular/cdk/portal': 'ng.cdk.portal', - '@angular/cdk/rxjs': 'ng.cdk.rxjs', - '@angular/cdk/table': 'ng.cdk.table', - '@angular/cdk': 'ng.cdk', - - // RxJS dependencies - 'rxjs/BehaviorSubject': 'Rx', - 'rxjs/Observable': 'Rx', - 'rxjs/Subject': 'Rx', - 'rxjs/Subscription': 'Rx', - 'rxjs/observable/combineLatest': 'Rx.Observable', - 'rxjs/observable/forkJoin': 'Rx.Observable', - 'rxjs/observable/fromEvent': 'Rx.Observable', - 'rxjs/observable/merge': 'Rx.Observable', - 'rxjs/observable/of': 'Rx.Observable', - 'rxjs/observable/throw': 'Rx.Observable', - 'rxjs/operator/auditTime': 'Rx.Observable.prototype', - 'rxjs/operator/catch': 'Rx.Observable.prototype', - 'rxjs/operator/debounceTime': 'Rx.Observable.prototype', - 'rxjs/operator/do': 'Rx.Observable.prototype', - 'rxjs/operator/filter': 'Rx.Observable.prototype', - 'rxjs/operator/finally': 'Rx.Observable.prototype', - 'rxjs/operator/first': 'Rx.Observable.prototype', - 'rxjs/operator/let': 'Rx.Observable.prototype', - 'rxjs/operator/map': 'Rx.Observable.prototype', - 'rxjs/operator/share': 'Rx.Observable.prototype', - 'rxjs/operator/startWith': 'Rx.Observable.prototype', - 'rxjs/operator/switchMap': 'Rx.Observable.prototype', - 'rxjs/operator/takeUntil': 'Rx.Observable.prototype', - 'rxjs/operator/toPromise': 'Rx.Observable.prototype', - - // RxJS imports for the examples package - 'rxjs/add/observable/merge': 'Rx.Observable', - 'rxjs/add/observable/fromEvent': 'Rx.Observable', - 'rxjs/add/observable/of': 'Rx.Observable', - 'rxjs/add/observable/interval': 'Rx.Observable', - 'rxjs/add/operator/startWith': 'Rx.Observable.prototype', - 'rxjs/add/operator/map': 'Rx.Observable.prototype', - 'rxjs/add/operator/debounceTime': 'Rx.Observable.prototype', - 'rxjs/add/operator/distinctUntilChanged': 'Rx.Observable.prototype', - 'rxjs/add/operator/first': 'Rx.Observable.prototype', - 'rxjs/add/operator/catch': 'Rx.Observable.prototype', - 'rxjs/add/operator/switchMap': 'Rx.Observable.prototype', -}; +const ROLLUP_GLOBALS = require('./rollup-globals.json'); export type BundleConfig = { entry: string; diff --git a/tools/tslint-rules/missingRollupGlobalsRule.js b/tools/tslint-rules/missingRollupGlobalsRule.js new file mode 100644 index 00000000000..8b591fdfbdd --- /dev/null +++ b/tools/tslint-rules/missingRollupGlobalsRule.js @@ -0,0 +1,43 @@ +const path = require('path'); +const Lint = require('tslint'); + +/** + * Rule that enforces that the specified external packages have been included in our Rollup config. + * Usage: [true, './path/to/rollup/config.json'] + */ +class Rule extends Lint.Rules.AbstractRule { + apply(file) { + return this.applyWithWalker(new Walker(file, this.getOptions())); + } +} + +class Walker extends Lint.RuleWalker { + constructor(file, options) { + super(...arguments); + + if (!options.ruleArguments.length) { + throw Error('missing-rollup-globals: The Rollup config path has to be specified.'); + } + + const [configPath, ...whitelist] = options.ruleArguments; + + this._configPath = path.resolve(process.cwd(), configPath); + this._config = require(this._configPath); + this._enabled = !whitelist.length || whitelist.some(p => new RegExp(p).test(file.fileName)); + } + + visitImportDeclaration(node) { + // Parse out the module name. The first and last characters are the quote marks. + const module = node.moduleSpecifier.getText().slice(1, -1); + const isExternal = !module.startsWith('.') && !module.startsWith('/'); + + // Check whether the module is external and whether it's in our config. + if (this._enabled && isExternal && !this._config[module]) { + this.addFailureAtNode(node, `Module "${module}" is missing from file ${this._configPath}.`); + } + + super.visitImportDeclaration(node); + } +} + +exports.Rule = Rule; diff --git a/tslint.json b/tslint.json index 2c9ee2c5518..ded87db2ace 100644 --- a/tslint.json +++ b/tslint.json @@ -35,6 +35,11 @@ "src/lib", "src/cdk" ], + "missing-rollup-globals": [ + true, + "./tools/package-tools/rollup-globals.json", + "^((?!tools).)*$" + ], "one-line": [ true, "check-catch",