Duplicate errors with noEmitOnError

[demurgos]

When the option notEmitOnError is set to true, gulp-typescript reports all the errors twice. This is specific to gulp-typescript, there is no issue when passing this option to tsc.

The easiest way to understand it is to see an example (see below for the source code):

Actual behavior:

Unique errors are reported twice when using the option noEmitOnError.

$ gulp
[23:47:11] Using gulpfile /data/projects/various/duplicate-ts-gulp/gulpfile.js
[23:47:11] Starting 'default'...
src/index.ts(1,7): error TS2322: Type 'number' is not assignable to type 'string'.
src/index.ts(1,7): error TS2322: Type 'number' is not assignable to type 'string'.
[23:47:12] TypeScript: 1 semantic error
[23:47:12] TypeScript: 1 emit error
[23:47:12] TypeScript: emit failed
[23:47:12] Finished 'default' after 889 ms

Expected behavior:

Unique errors should only be reported once.

$ gulp
[23:47:11] Using gulpfile /data/projects/various/duplicate-ts-gulp/gulpfile.js
[23:47:11] Starting 'default'...
src/index.ts(1,7): error TS2322: Type 'number' is not assignable to type 'string'.
[23:47:12] TypeScript: 1 semantic error
[23:47:12] TypeScript: 1 emit error
[23:47:12] TypeScript: emit failed
[23:47:12] Finished 'default' after 889 ms

---

You can easily reproduce this error with the three following files (or just clone this temporary repo):

package.json

{
  "name": "duplicate-ts-gulp",
  "version": "1.0.0",
  "private": true,
  "devDependencies": {
    "gulp": "^3.9.1",
    "gulp-typescript": "^3.2.3",
    "typescript": "^2.5.3"
  }
}

gulpfile.js

var gulp = require('gulp');
var ts = require('gulp-typescript');

gulp.task('default', function () {
  return gulp.src('src/**/*.ts')
    .pipe(ts({noEmitOnError: true}))
    .pipe(gulp.dest('build'));
}); 

src/index.ts

const foo: string = 1 + 1;

[demurgos]

The problem occurs here:

gulp-typescript/lib/compiler.ts

Lines 144 to 151 in 6914e22

	private emit(result: CompilationResult, callback: ts.WriteFileCallback) {
	const emitOutput = this.program.emit(undefined, callback);
	result.emitErrors += emitOutput.diagnostics.length;
	this.reportDiagnostics(emitOutput.diagnostics);
	result.emitSkipped = emitOutput.emitSkipped;
	}

With {noEmitOnError: false}, I get the following output for emitOutput:

{ emitSkipped: false,
  diagnostics: [],
  emittedFiles: undefined,
  sourceMaps: undefined }

But with {noEmitOnError: true}, I get:

{ diagnostics: 
   [ { file: [Object],
       start: 6,
       length: 3,
       code: 2322,
       category: 1,
       messageText: 'Type \'1\' is not assignable to type \'string\'.' } ],
  sourceMaps: undefined,
  emittedFiles: undefined,
  emitSkipped: true }

[demurgos]

I pushed a test-case demonstrating the issue on my branch issue-543. I added a test directory called noEmitOnError: this test is currently failing because of the duplicate emit.

A possible explanation is that the error seems to be counted both as a semantic error and emit error.

[ivogabe]

Thanks for reporting and investigating this issue. I think this is an issue of TypeScript, I do not think that we should remove duplicate errors ourselves in gulp-typescript. So I reported microsoft/TypeScript#20876, and hope that we'll get a response from them soon.

[demurgos]

The related TS issue was closed by this PR.

[demurgos]

Thanks for fixing this!