Changed default from ES3 to ES5

[perlun]

Fixes #10117

The fix was easy, but then I had to get the tests passing:

$ gulp runtests # loads of failures, more than 1000 actually.
$ gulp baseline-accept
$ gulp runtests # to make sure they now passed

...and then git add . to add all the changes. (There were a bunch of new files created when I did it like this; is that correct?)

---

(Regarding the unrelated changes in compileOnSave.ts: I happened to see a few typing mistakes there very close to the changes I were making to that files, so I bundled that change along at the same time. Since the tests are passing I think it should be fine.)

[msftclas]

@perlun,
Thanks for having already signed the Contribution License Agreement. Your agreement was validated by Microsoft. We will now review your pull request.
Thanks,
Microsoft Pull Request Bot

[perlun]

Argh, because of the delay the PR has gotten into merge conflicts. 😛 Will rebase and let you know once this is reviewable again.

[perlun]

@RyanCavanaugh or @ahejlsberg - this one is rebased again on top of the current master, and seems to build cleanly.

Would highly appreciate it we could get someone to look at it; because of the huge number of files affected (1450...), it is highly likely that it will run into merge conflicts over and over again if we leave it for a long time. Thanks in advance.

[DanielRosenwasser]

Hi @perlun, sorry for the delay. I'm currently a little wary of which files were intended to be compiled on ES3 vs which files just happen to have minor output baselines changed. @mhegazy and @RyanCavanaugh may have some better guidance here.

[saschanaz]

I think adding @target: es3 for all compiler tests is one of the safest option here. (This will give no baseline change)

[perlun]

@saschanaz Sounds doable. I am not an expert in this codebase, so please give some more details and I'll see if I can get it sorted out. Where should this be added more specifically?

[saschanaz]

@perlun It should be added as // @target: es3 on the top of each compiler test file. For example, tests/cases/compiler/2dArrays.ts will become:

// @target: es3
class Cell {
}

class Ship {
    isSunk: boolean;
}

class Board {
    ships: Ship[];
    cells: Cell[];

    private allShipsSunk() {
        return this.ships.every(function (val) { return val.isSunk; });
    }    
}

I would use some automated script to do this.

[perlun]

Thanks a lot @saschanaz, that sounds doable. I couldn't find tests/cases/compiler/2dArrays.ts though, only ./tests/baselines/reference/2dArrays.js

Would the @target stuff come before or after the four slashes - I guess after of course? But then that makes it a lot harder to automate unfortunately...

[saschanaz]

I couldn't find tests/cases/compiler/2dArrays.ts though

The file is here: https://github.com/Microsoft/TypeScript/blob/master/tests/cases/compiler/2dArrays.ts

Would the @target stuff come before or after the four slashes - I guess after of course?

Compiler tests does not use four slashes, you can just add one on the top of the file. Fourslash tests don't support @lib directive yet. (17b5efb adds the support.) Anyway the answer is fortunately 'before'. 😄

[perlun]

The file is here: https://github.com/Microsoft/TypeScript/blob/master/tests/cases/compiler/2dArrays.ts

Argh! I searched for it, but for whatever reason did a find -name 2dArrays.js instead of .ts... 🤦‍♂️

Thanks a lot! Will try to get this one sorted out within the next week or so.

[mhegazy]

Had a discussion about this change in the design meeting today, you can find more details in #10117 (comment).

as noted in #10117 (comment), we will not be taking this change in the time being. sorry for the delay, and the miss labeling of the issue as committed.