gulp-typescript: crash the build process on error

Expected behavior: When a typescript error is detected, gulp shouldn’t exit with code 0, an error should be reported instead.

Actual behavior: errors are displayed but gulp exists with code 0.

Your gulpfile:

Include your gulpfile, or only the related task (with ts.createProject).

var tsProject = ts.createProject('tsconfig.json', {
  typescript: require('typescript'),
});
gulp.task('ts', function() {
  return gulp.src([
    'typings/**/*.d.ts',
    'server*.ts',
    'webpack.config.ts',
    'app/**/*.ts',
    'app/**/*.tsx',
  ], { base: __dirname })
    .pipe(sourcemaps.init({
      loadMaps: true,
    }))
    .pipe(ts(tsProject))
    .pipe(sourcemaps.write('.'))
    .pipe(gulp.dest('.'));
});

tsconfig.json

Include your tsconfig, if related to this issue.

{
  "compilerOptions": {
    "target": "es2015",
    "module": "commonjs",
    "jsx": "preserve",
    "inlineSourceMap": true,
    "moduleResolution": "node",
    "experimentalDecorators": true
  },
  "exclude": [
    "node_modules"
  ]
}

About this issue

  • Original URL
  • State: closed
  • Created 8 years ago
  • Reactions: 24
  • Comments: 28 (6 by maintainers)

Commits related to this issue

Most upvoted comments

I was hoping this would be a flag or something. This is a very common use case as it makes no sense to ignore errors in production builds/CI environment.

simplified

  .once("error", function () {
    this.once("finish", () => process.exit(1));
  })

@danielmoore please use Github’s “thumbs up” reaction feature instead of “+1” comments. Reactions are designed to be a better alternative for expressing support without the visual clutter and additional email notifications that “+1” comments bring.

I’m just adding that gulp-less has the same problem, so I agree it should be fixed upstream in the main gulp project. Because our less build step failed, the site was broken, but gulp still “succeeded”, so the deploy succeeded.

I think developers would agree that while “breaking watch” is annoying, breaking production deploys because broken build steps “succeed” is a worse default behavior!

Most users don’t want this, since it would break their watch task. However, you can use something like this to exit with a specific code:

    ...
    .pipe(ts(tsProject))
    .on('error', function() { process.exit(1) })

I added the following to my gulpfile.js and it seemed to work:

let failed = false;
...
  .pipe(ts(tsProject))
  .on("error", function () { failed = true; })
  .on("finish", function () { failed && process.exit(1); });

In my opinion this should not be integrated directly into gulp-typescript; but I do think that it would be a good idea to include a demonstration of this use case in the documentation.

The fix on package consumer side is really concise, which I’m happy about, but I will say that a lot of other linters and gulp plugins will come with an optional way to fail the task if “error conditions” were detected (like warnings / errors in a linter), which can then be opted into as the task implementer sees fit in their CI builds.

gulp-jshint has a pretty good solution to this: a ‘fail’ reporter

stuff
  .pipe(jshint())
  .pipe(jshint.reporter('jshint-stylish'))
  .pipe(jshint.reporter('fail'))

Here is my solution. It works fine for me, and it don’t break watch!

Use async task because I want to end this task conditionally. I put error handler on tsResult to count the number of errors and finish handler after dest pipe to delete created files if there is any error and send a error to the callback to break gulp tasks, otherwise I call the callback without any argument to continue remaining tasks.

const gulp = require("gulp");
const del = require("del");
const ts = require("gulp-typescript");

gulp.task('compile', function (cb) {
    let errorCount = 0;
    const tsProject = ts.createProject("./tsconfig.json");
    const tsResult = gulp
        .src("./src/**/*.ts")
        .pipe(tsProject())
        .on("error", () => {
            errorCount += 1;
        });

    tsResult.js
        .pipe(gulp.dest("./build"))
        .on("finish", () => {
            if (errorCount > 0) {
                let error = new Error(`Failed to compile: ${errorCount} error(s).`);
                error['showStack'] = false;

                del("./build/**").then(() => {
                    cb(error);
                });
            } else {
                cb();
            }
        });

    return gulp.src('src/**/*.ts')
        .pipe(ts({
            noImplicitAny: true,
            out: 'output.js'
        }))
        .pipe(gulp.dest('built/local'));
});

The crashing behavior will be part of our release 5 🎉! Thanks for everyones opinions, I tested the behavior of gulp 4 and the watch scenario works there. The changes with the current behavior are as follows:

  • gulp-typescript does not add a noop event listener to the error event. Errors will thus crash the build.
  • gulp-typescript emits only a single error. It appeared that the new watch behavior of gulp 4 only catches the first error. We thus emit a single error at the end of the compilation. To get notified of each separate error, one should use a reporter.
  • As pointed by @phated, gulp does not crash in watch mode.

To prevent crashing the build, even outside of watch mode, you can add .on('error', () => {}) to your stream, just after .pipe(tsProject()).

You can install the new version with npm install gulp-typescript@5.0.0-alpha.1

Most users don’t want this, since it would break their watch task.

@ivogabe I think you should reconsider breaking this for watch tasks. The default invocation of gulp-typescript should be conservative rather than permissive for a use-case (watch) that many people may not even utilize. Rather than new users failing to notice that errors are not failing gulp (“you don’t know what you don’t know”) I would think it would be easier for users attempting to setup a watch task to fail and go looking for an answer.

I still don’t want to break this, especially given that the philosophy of TypeScript is that all errors are warnings, and there are no fatal errors. Though to make easier to crash the build, I’d suggest to add a new reporter (see readme) which will crash in case of errors. That way there is less code needed to configure this. Do you agree on this solution?

We recently had to deal with this issue as well. We chose to invoke a callback function of the task with an error message. This would look something like this:

gulp.task('ts', function(next) {
  return gulp.src([
    'typings/**/*.d.ts',
    'server*.ts',
    'webpack.config.ts',
    'app/**/*.ts',
    'app/**/*.tsx',
  ], { base: __dirname })
    .pipe(sourcemaps.init({
      loadMaps: true,
    }))
    .pipe(ts(tsProject))
    .on('error', next('Failed during typescript compilation'))
    .pipe(sourcemaps.write('.'))
    .pipe(gulp.dest('.'));
});

@ivogabe If we really want to keep the current behavior (opt-in non-zero exit code on errors), I think there should be a note about that on the README page (+ a hint for a workaround, e.g. this one).

That said, I would still prefer opt-out (for watch tasks).

I think that’s more suitable to be implemented directly in gulp instead of gulp-typescript. Can you report it there? Other plugins could also benefit from this.