Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider changing default target to ES5 #10117

Closed
RyanCavanaugh opened this issue Aug 3, 2016 · 8 comments
Closed

Consider changing default target to ES5 #10117

RyanCavanaugh opened this issue Aug 3, 2016 · 8 comments
Assignees
Labels
Declined The issue was declined as something which matches the TypeScript vision Suggestion An idea for TypeScript

Comments

@RyanCavanaugh
Copy link
Member

ES3 runtimes are increasingly few and far between, and many common features (getters and setters, spread in new) only work in ES5. The 2.0 release would be a good time to take a BC break to update the default target to ES5.

Related, and somewhat confusingly, we emit a tsconfig.json with target: ES5 when running tsc --init even though the actual default target is ES3. We should at least be consistent here.

@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript In Discussion Not yet reached consensus labels Aug 3, 2016
@unional
Copy link
Contributor

unional commented Aug 3, 2016

One small counter argument. When I first get my hand on TypeScript, it is the ability to transpile down to ES3 that gets me to choose TypeScript over ES6 with Babel.

Of course things changed since then and more and more features are added.

So my personal opinion on this is mixed and neutral. 🌷

@RyanCavanaugh
Copy link
Member Author

We'd still allow ES3 as a target, just not have it be the default.

@unional
Copy link
Contributor

unional commented Aug 3, 2016

Oh. I thought we are dropping ES3. XD

All for it then. :)

@RyanCavanaugh RyanCavanaugh added Help Wanted You can do this Committed The team has roadmapped this issue and removed In Discussion Not yet reached consensus labels Aug 22, 2016
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 2.1 milestone Aug 22, 2016
@RyanCavanaugh
Copy link
Member Author

RyanCavanaugh commented Aug 22, 2016

👍 for TS 2.1. THE FUTURE IS NOW (eventually)

@jcrben
Copy link

jcrben commented Apr 8, 2017

Looks like the milestone should be changed here, although target ES5 got set as the default for tsc ---init. fyi, applicable code is at https://github.com/Microsoft/TypeScript/blob/2b61d18996c182b7de8ef2a9367210a70e3d4031/src/compiler/core.ts#L1458-L1474

defaults are apparently in various places; e.g., charset is at https://github.com/Microsoft/TypeScript/blob/0fd0903280d9c5090f18bcc22b7df7ca608857b1/src/compiler/sys.ts#L196-L223

seems worthwhile to move towards handling them in one place

perlun added a commit to perlun/TypeScript that referenced this issue Apr 29, 2017
Fixes microsoft#10117

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

```shell
$ 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.)
perlun added a commit to perlun/TypeScript that referenced this issue Apr 29, 2017
Fixes microsoft#10117

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

```shell
$ 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.)
perlun added a commit to perlun/TypeScript that referenced this issue Apr 29, 2017
Fixes microsoft#10117

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

```shell
$ 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.)
@perlun
Copy link

perlun commented Apr 29, 2017

@RyanCavanaugh - see #15466.

perlun added a commit to perlun/TypeScript that referenced this issue May 10, 2017
Fixes microsoft#10117

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

```shell
$ 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.)
perlun added a commit to perlun/TypeScript that referenced this issue May 31, 2017
(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.)
@mhegazy
Copy link
Contributor

mhegazy commented Jun 2, 2017

Had a discussion about this again in the design meeting today. There seems to be little benefit from the change since it basically saves you from writing --target es5. on the other hand, the change includes a breaking change for existing customers who do not have --target specified.

It gets even more interesting that the breaking change is not easily detectable, i.e. the emit changes silently with no warnings, and users will have to figure this out the hard way. where as the current behavior of asking users to set --target ES5 to use a setter/getter is straight forward.

The other ideological issues here is there does not seem to be a principal to choosing the default, ES3 is the minimum supported. ES5 is some value in the middle (though granted is the most common), but that means that we will do another breaking change in a few years to pump it to ES6 when that gets widely used enough, and it is not clear where does that stop.

So to conclude, do not think we will do this after all. sorry for flip flopping on it.

@mhegazy mhegazy closed this as completed Jun 2, 2017
@mhegazy mhegazy added Declined The issue was declined as something which matches the TypeScript vision and removed Help Wanted You can do this labels Jun 2, 2017
@mhegazy mhegazy removed the Committed The team has roadmapped this issue label Jun 2, 2017
@perlun
Copy link

perlun commented Jun 2, 2017

Thanks for the details @mhegazy. My suggestion:

  • Go with what you already settled with for the rest of the 2.x series, your reasoning totally makes sense.
  • Consider this for inclusion in 3.0, where there is room for breaking changes and this can also be clearly communicated ("new baseline: ES5"). Of course, depending on when 3.0 will happen, the new baseline can be set to either ES5 or ES6.

I think it's fine to go with first ES5 and then ES6, as long as it's clearly communicated, but of course it means a bit more work for you guys. Semantic Versioning is the agreed-upon contract within the industry for handling these kind of things. In line with SemVer, I think your conclusion for 2.x is entirely right: we must not take this change in, since it's very likely that it will cause hard-to-find bugs and make people rightfully upset. They should be able to trust that 2.x will not introduce changes like this.

So, I don't have any problem with this decision, as long as we are clear that it's temporal. (while "temporal" can mean anything from months to years...)

@mhegazy mhegazy removed this from the Future milestone Apr 26, 2018
@microsoft microsoft locked and limited conversation to collaborators Jul 31, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Declined The issue was declined as something which matches the TypeScript vision Suggestion An idea for TypeScript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants