GeezerVison

I"m not sure what the purpose of this is. Do we have any actual warnings in typescript today?

Cyrus Najmabadi (@CyrusNajmabadi) Not real warnings but when would type this:

var foo: number = "test";

It would still create a file with output with valid JavaScript. I added this piece of code because the issue I linked to is having the label "Accepting PR".
DanQuirk is right about the EmitReturnStatus enum so this code might not be really needed but than I think that issue should be closed as well?

I think we need that for some scenarios. Incremental build with errors for instance does not work wiper simthing like this.

I do not like the name of the flag; users will have the same reaction as Cyrus Najmabadi (@CyrusNajmabadi). I can not find a good name though, may be suppressEmitWithErrors or early..

I'm also not big on the name, but I'm at a loss for significantly better ones.

• emitOnCleanBuild
• emitCleanBuildsOnly
• noEmitOnAnyErrors

I like (or at least tolerate better) noEmitOnError[s] or emitCleanOnly. We're consistent in terminology about type errors being "errors" and not "warnings".

yeah I think noEmitOnErrors is sufficiently clear and concise

The logic on whether to emit or not is used in three places, 1. tsc.ts (batch compiler), and you got that, 2. emitter.ts (used by compile on save), and 3. harness.ts (used by the tests to mimic the batch compiler behavior). we need to ensure that all 3 do the same thing.
I would change the name of hasEarlyError to isEmitBlocked and add the check for the option and the error as well. i would also check global errors and syntax errors (program.getDiagnostics().length === 0) to be safe.

something like this:

function isEmitBlocked(sourceFile?: SourceFile): boolean {
    return program.getDiagnostics(sourceFile).length !== 0 ||
        hasEarlyErrors(sourceFile) ||
        (compilerOptions.noEmitOnError  && getDiagnostics(sourceFile));
}

Note: sourceFile is needed here, but it is optional. in case of batch compiler, sourceFile is not specified, as we want to do the check globally. but for compile on save, we want to do the check on one file only.

One more thing, we will need a unit test for this. Unit tests use reads "// @: value" from the beginning of the file, and uses them to set compilation settings. look at the switch statement in: https://github.com/Microsoft/TypeScript/blob/master/src/harness/harness.ts#L672

Then add a test that has "// @noEmitOnErrors: true" and ensure that there are no outputs. i would also add a Sourcemap (@sourcemap) and declaration

Added some new commits.
Also renamed the hasEarlyErrors method in the interface to is isEmitBlocked (which is what you meant right? or does this break things with VS maybe?)
Changed some things in harness.ts to make it work.

For future reference the link to harness with SHA in the url harness.ts. (You can create this url by clicking on a line number and press 'y' and it will always be the right one)

Comments for isEarly and hasEarlyError will make it clearer.

Thanks for feedback everyone. Will add some comments in compilerRunner.ts
Yui (@yuit) Comments for isEarly and hasEarlyError will make it clearer
Sorry, don't know what you mean here (where do you want such comments?)

For some reasons I can't see where you defined hasEarlyError in the change list. However, what I mean is that could you add comment at the when you declare function hasEarlyError and property isEarly in the Diagnostics. I am not sure what is the property is for

hasEarlyError was already there, the messages which have isEarly are in diagnosticMessages.json
I'm actually not really sure why those have isEarly so I can't really add a comment for it.

👍

Dick van den Brink (@DickvdBrink) extremely sorry for the delay. This totally fell off my radar. The PR is ready to go, can you refresh the PR by merging from main.

Mohamed Hegazy (@mhegazy) no problem, better late than never right ;)
Updated PR and after the merge I ran the tests again and noticed that my baseline had changed so fixed that in another commit

thanks, and sorry again for the delay.