Skip to content
This repository was archived by the owner on May 1, 2020. It is now read-only.

Commit b3bb906

Browse files
committed
feat(lint): new option to have stand alone lint bail
new option to have stand alone lint bail
1 parent 4055d73 commit b3bb906

File tree

7 files changed

+115
-44
lines changed

7 files changed

+115
-44
lines changed

README.md

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -124,8 +124,7 @@ npm run build --rollup ./config/rollup.config.js
124124
| output js map file | `ionic_output_js_map_file_name` | `--outputJsMapFileName` | `main.js.map` | name of js source map file generated in `buildDir` |
125125
| output css file | `ionic_output_css_file_name` | `--outputCssFileName` | `main.css` | name of css file generated in `buildDir` |
126126
| output css map file | `ionic_output_css_map_file_name` | `--outputCssMapFileName` | `main.css.map` | name of css source map file generated in `buildDir` |
127-
128-
127+
| bail on lint error | `ionic_bail_on_lint_error` | `--bailOnLintError` | `null` | Set to `true` to make stand-alone lint commands fail with non-zero status code |
129128

130129

131130

@@ -157,7 +156,7 @@ These environment variables are automatically set to [Node's `process.env`](http
157156
| `IONIC_OUTPUT_CSS_MAP_FILE_NAME` | The file name of the generated css source map file |
158157
| `IONIC_WEBPACK_FACTORY` | The absolute path to Ionic's `webpack-factory` script |
159158
| `IONIC_WEBPACK_LOADER` | The absolute path to Ionic's custom webpack loader |
160-
159+
| `IONIC_BAIL_ON_LINT_ERROR` | Boolean determining whether to exit with a non-zero status code on error |
161160

162161

163162
The `process.env.IONIC_ENV` environment variable can be used to test whether it is a `prod` or `dev` build, which automatically gets set by any command. By default the `build` and `serve` tasks produce `dev` builds (a build that does not include Ahead of Time (AoT) compilation or minification). To force a `prod` build you should use the `--prod` command line flag.
@@ -190,6 +189,19 @@ Example NPM Script:
190189

191190
## Tips
192191
1. The Webpack `devtool` setting is driven by the `ionic_source_map_type` variable. It defaults to `source-map` for the best quality source map. Developers can enable significantly faster builds by setting `ionic_source_map_type` to `eval`.
192+
2. By default, the `lint` command does not exit with a non-zero status code on error. To enable this, pass `--bailOnLintError true` to the command.
193+
194+
```
195+
"scripts" : {
196+
...
197+
"lint": "ionic-app-scripts lint"
198+
...
199+
}
200+
```
201+
202+
```
203+
npm run lint --bailOnLintError true
204+
```
193205

194206

195207
## The Stack

src/build.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ function buildProject(context: BuildContext) {
123123
.then(() => {
124124
// kick off the tslint after everything else
125125
// nothing needs to wait on its completion
126-
lint(context);
126+
return lint(context);
127127
})
128128
.catch(err => {
129129
throw new BuildError(err);

src/lint.spec.ts

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
import * as lint from './lint';
2+
import * as workerClient from './worker-client';
3+
import * as Constants from './util/constants';
4+
5+
let originalEnv = process.env;
6+
7+
describe('lint task', () => {
8+
describe('lint', () => {
9+
10+
beforeEach(() => {
11+
originalEnv = process.env;
12+
process.env = {};
13+
});
14+
15+
afterEach(() => {
16+
process.env = originalEnv;
17+
});
18+
19+
it('Should return resolved promise', (done: Function) => {
20+
// arrange
21+
spyOn(workerClient, workerClient.runWorker.name).and.returnValue(Promise.resolve());
22+
// act
23+
const promise = lint.lint(null);
24+
25+
// assert
26+
promise.then(() => {
27+
done();
28+
});
29+
});
30+
31+
it('Should return resolved promise when bail on error is not set', (done: Function) => {
32+
// arrange
33+
spyOn(workerClient, workerClient.runWorker.name).and.returnValue(Promise.reject(new Error('Simulating an error')));
34+
// act
35+
const promise = lint.lint(null);
36+
37+
// assert
38+
promise.then(() => {
39+
done();
40+
});
41+
});
42+
43+
it('Should return rejected promise when bail on error is set', (done: Function) => {
44+
45+
spyOn(workerClient, workerClient.runWorker.name).and.returnValue(Promise.reject(new Error('Simulating an error')));
46+
process.env[Constants.ENV_BAIL_ON_LINT_ERROR] = 'true';
47+
48+
// act
49+
const promise = lint.lint(null);
50+
51+
// assert
52+
promise.catch(() => {
53+
done();
54+
});
55+
});
56+
});
57+
});

src/lint.ts

Lines changed: 33 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ import { BuildContext, ChangedFile, TaskInfo } from './util/interfaces';
33
import { BuildError } from './util/errors';
44
import { createProgram, findConfiguration, getFileNames } from 'tslint';
55
import { getUserConfigFile } from './util/config';
6+
import * as Constants from './util/constants';
7+
import { readFileAsync, getBooleanPropertyValue } from './util/helpers';
68
import { join } from 'path';
79
import { Logger } from './logger/logger';
810
import { printDiagnostics, DiagnosticsType } from './logger/logger-diagnostics';
@@ -14,9 +16,17 @@ import * as ts from 'typescript';
1416

1517

1618
export function lint(context: BuildContext, configFile?: string) {
19+
const logger = new Logger('lint');
20+
console.log('process.env: ', process.env);
1721
return runWorker('lint', 'lintWorker', context, configFile)
22+
.then(() => {
23+
logger.finish();
24+
})
1825
.catch(err => {
19-
throw new BuildError(err);
26+
if (getBooleanPropertyValue(Constants.ENV_BAIL_ON_LINT_ERROR)){
27+
throw logger.fail(err);
28+
}
29+
logger.finish();
2030
});
2131
}
2232

@@ -25,7 +35,7 @@ export function lintWorker(context: BuildContext, configFile: string) {
2535
return getLintConfig(context, configFile).then(configFile => {
2636
// there's a valid tslint config, let's continue
2737
return lintApp(context, configFile);
28-
}).catch(() => {});
38+
});
2939
}
3040

3141

@@ -62,7 +72,7 @@ function lintApp(context: BuildContext, configFile: string) {
6272
return lintFile(context, program, file);
6373
});
6474

65-
return Promise.all(promises);
75+
return Promise.all(promises)
6676
}
6777

6878
function lintFiles(context: BuildContext, program: ts.Program, filePaths: string[]) {
@@ -74,45 +84,28 @@ function lintFiles(context: BuildContext, program: ts.Program, filePaths: string
7484
}
7585

7686
function lintFile(context: BuildContext, program: ts.Program, filePath: string): Promise<any> {
77-
return new Promise((resolve) => {
78-
87+
return Promise.resolve().then(() => {
7988
if (isMpegFile(filePath)) {
80-
// silly .ts files actually being video files
81-
resolve();
82-
return;
89+
throw new Error(`${filePath} is not a valid TypeScript file`);
8390
}
84-
85-
fs.readFile(filePath, 'utf8', (err, contents) => {
86-
if (err) {
87-
// don't care if there was an error
88-
// let's just move on with our lives
89-
resolve();
90-
return;
91-
}
92-
93-
try {
94-
const configuration = findConfiguration(null, filePath);
95-
96-
const linter = new Linter(filePath, contents, {
97-
configuration: configuration,
98-
formatter: null,
99-
formattersDirectory: null,
100-
rulesDirectory: null,
101-
}, program);
102-
103-
const lintResult = linter.lint();
104-
if (lintResult && lintResult.failures) {
105-
const diagnostics = runTsLintDiagnostics(context, <any>lintResult.failures);
106-
printDiagnostics(context, DiagnosticsType.TsLint, diagnostics, true, false);
107-
}
108-
109-
} catch (e) {
110-
Logger.debug(`Linter ${e}`);
111-
}
112-
113-
resolve();
114-
});
115-
91+
return readFileAsync(filePath);
92+
}).then((fileContents: string) => {
93+
const configuration = findConfiguration(null, filePath);
94+
95+
const linter = new Linter(filePath, fileContents, {
96+
configuration: configuration,
97+
formatter: null,
98+
formattersDirectory: null,
99+
rulesDirectory: null,
100+
}, program);
101+
102+
const lintResult = linter.lint();
103+
if (lintResult && lintResult.failures && lintResult.failures.length) {
104+
const diagnostics = runTsLintDiagnostics(context, <any>lintResult.failures);
105+
printDiagnostics(context, DiagnosticsType.TsLint, diagnostics, true, false);
106+
throw new BuildError(`${filePath} did not pass TSLint`);
107+
}
108+
return lintResult;
116109
});
117110
}
118111

src/util/config.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,9 @@ export function generateContext(context?: BuildContext): BuildContext {
120120
const aotWriteToDisk = getConfigValue(context, '--aotWriteToDisk', null, Constants.ENV_AOT_WRITE_TO_DISK, Constants.ENV_AOT_WRITE_TO_DISK.toLowerCase(), null);
121121
setProcessEnvVar(Constants.ENV_AOT_WRITE_TO_DISK, aotWriteToDisk);
122122

123+
const bailOnLintError = getConfigValue(context, '--bailOnLintError', null, Constants.ENV_BAIL_ON_LINT_ERROR, Constants.ENV_BAIL_ON_LINT_ERROR.toLowerCase(), null);
124+
setProcessEnvVar(Constants.ENV_BAIL_ON_LINT_ERROR, bailOnLintError);
125+
123126
if (!isValidBundler(context.bundler)) {
124127
context.bundler = bundlerStrategy(context);
125128
}

src/util/constants.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ export const ENV_OUTPUT_CSS_MAP_FILE_NAME = 'IONIC_OUTPUT_CSS_MAP_FILE_NAME';
3838
export const ENV_WEBPACK_FACTORY = 'IONIC_WEBPACK_FACTORY';
3939
export const ENV_WEBPACK_LOADER = 'IONIC_WEBPACK_LOADER';
4040
export const ENV_AOT_WRITE_TO_DISK = 'IONIC_AOT_WRITE_TO_DISK';
41+
export const ENV_BAIL_ON_LINT_ERROR = 'IONIC_BAIL_ON_LINT_ERROR';
4142

4243
export const BUNDLER_ROLLUP = 'rollup';
4344
export const BUNDLER_WEBPACK = 'webpack';

src/util/helpers.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,3 +217,8 @@ export function stringSplice(source: string, startIndex: number, numToDelete: nu
217217
export function toUnixPath(filePath: string) {
218218
return filePath.replace(/\\/g, '/');
219219
}
220+
221+
export function getBooleanPropertyValue(propertyName: string) {
222+
const result = process.env[propertyName];
223+
return result === 'true';
224+
}

0 commit comments

Comments
 (0)