Add ES2017 target by andrejbaran · Pull Request #11407 · microsoft/TypeScript

andrejbaran · 2016-10-05T23:22:42Z

Hi there,
please consider this as a start for ES2017 target. Fixes #10768.

10/13/2016: This PR also includes a consolidation of naming of ES versions to reflect the official naming scheme (past es5) ES_YYYY_. This is a change through out the codebase.

msftclas · 2016-10-05T23:22:47Z

Hi @andrejbaran, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by Microsoft and real humans are currently evaluating your PR.

TTYL, MSBOT;

mhegazy · 2016-10-05T23:55:08Z

src/compiler/commandLineParser.ts

                "es3": ScriptTarget.ES3,
                "es5": ScriptTarget.ES5,
                "es6": ScriptTarget.ES6,
+                "es8": ScriptTarget.ES8,


Officially there is no ES8, it is ES2017, so i would not add ES8 here or anywhere else. i would just leave it to ES2017

mhegazy · 2016-10-05T23:58:44Z

src/compiler/commandLineParser.ts

                "es3": ScriptTarget.ES3,
                "es5": ScriptTarget.ES5,
                "es6": ScriptTarget.ES6,
+                "es8": ScriptTarget.ES8,


Can you also add ES2016. it is a bit funny that we skip one ES 2016 (though not very useful but should be there for consistency).

mhegazy · 2016-10-05T23:59:13Z

src/compiler/commandLineParser.ts

                "es3": ScriptTarget.ES3,
                "es5": ScriptTarget.ES5,
                "es6": ScriptTarget.ES6,
+                "es8": ScriptTarget.ES8,


you also need to update getDefaultLibFileName to inject the right library based on the target.

mhegazy · 2016-10-06T00:02:11Z

src/compiler/transformers/ts.ts

@@ -2453,14 +2464,28 @@ namespace ts {
         * @param node The await expression node.
         */
        function visitAwaitExpression(node: AwaitExpression): Expression {


async/await should be split into its own transform, es2017.ts, then we do not need to run it in this case, just like ES7 transform.

mhegazy · 2016-10-06T00:12:22Z

thanks @andrejbaran for the contribution. a few comments. also cc @rbuckton

yortus · 2016-10-06T04:45:38Z

In theory trailing commas should also be preserved in the output for fidelity with target: es2017. Is that being too pedantic? I can't imagine any scenarios where it would matter.

mhegazy · 2016-10-06T05:05:28Z

In theory trailing commas should also be preserved in the output for fidelity with target: es2017. Is that being too pedantic? I can't imagine any scenarios where it would matter.

yes, but the implementation today would make this a bit hard to achieve, not sure if there is much value though.

yortus · 2016-10-06T05:27:09Z

I can't imagine any scenarios where it would matter.

Thinking about it more, at least one scenario comes to mind: ES feature detection. Not sure how useful that is in practice (NB the linked repo has 0 stars). Probably safe to ignore until someone raises an actual issue.

andrejbaran · 2016-10-06T13:45:33Z

Thanks for the review @mhegazy i'll get back to it tonight and will let you know

Rename es7 to es2016. Update getDefaultLibFileName for new targets.

andrejbaran · 2016-10-12T20:20:40Z

@mhegazy It turned out to be little more complicated than I anticipated, please review again. I have also renamed es7 to es2016 for consistency? (maybe es6 should be renamed too, dunno but I believe es2015 is preferred)

sandersn

Looks good except for the binder marking async/await as Typescript even though I think they should just be ES2017 now. @rbuckton can you take a look at that?

sandersn · 2016-10-12T20:46:24Z

src/compiler/binder.ts

            // If a FunctionDeclaration is async, then it is TypeScript syntax.
            if (modifierFlags & ModifierFlags.Async) {
-                transformFlags |= TransformFlags.AssertTypeScript;
+                transformFlags |= TransformFlags.AssertTypeScript | TransformFlags.AssertES2017;


is this comment still accurate? Or are async functions valid ES2017 too? If so, they probably shouldn't be marked as typescript.

According to this async/await is finished and will be part of es2017, so I believe you have a valid point there.

sandersn · 2016-10-12T20:46:40Z

src/compiler/binder.ts

        // An async function expression is TypeScript syntax.
        if (modifierFlags & ModifierFlags.Async) {
-            transformFlags |= TransformFlags.AssertTypeScript;
+            transformFlags |= TransformFlags.AssertTypeScript | TransformFlags.AssertES2017;


same comment as for async function declarations

sandersn · 2016-10-12T20:47:15Z

src/compiler/binder.ts

        switch (kind) {
+            case SyntaxKind.AsyncKeyword:
+            case SyntaxKind.AwaitExpression:
+                // Typescript async/await are ES2017 features


same comment as elsewhere. probably should just be AssertES2017

sandersn · 2016-10-12T20:48:19Z

src/compiler/transformers/es2017.ts

+/// <reference path="../visitor.ts" />
+
+/*@internal*/
+namespace ts {


@rbuckton do you mind taking a look at least at this file and ts.ts?

sandersn · 2016-10-12T20:49:12Z

src/compiler/transformers/ts.ts


                case SyntaxKind.AwaitExpression:
-                    // TypeScript 'await' expressions must be transformed.
+                    // Typescript ES2017 async/await are handled by ES2017 transformer


I don't think you will even need this case if you mark async/await as ES2017 only

sandersn · 2016-10-12T20:49:34Z

src/compiler/transformers/ts.ts

-            if (isAsyncFunctionLike(node)) {
-                return <FunctionBody>transformAsyncFunctionBody(node);
-            }
+            // if (isAsyncFunctionLike(node) && languageVersion < ScriptTarget.ES2017) {


delete this and the next comment section

rbuckton · 2016-10-13T00:12:42Z

src/compiler/binder.ts

        // extends clause of a class.
        let transformFlags = subtreeFlags | TransformFlags.AssertES6;

+        // propagate ES2017


Any TransformFlags of a subtree that should propagate should already be set in subtreeFlags. There should be no need to explicitly propagate.

rbuckton · 2016-10-13T00:14:24Z

src/compiler/binder.ts

        }

+        // Async MethodDeclaration is ES2017
+        if (modifierFlags & ModifierFlags.Async) {


Since async is ES2017, the test at line 2598 is now incorrect.

rbuckton · 2016-10-13T00:19:31Z

src/compiler/transformers/es2017.ts

+                    return undefined;
+
+                case SyntaxKind.AwaitExpression:
+                    // Typescript 'await' expressions must be transformed for targets < ES2017.


Should be ES2017 and not Typescript here

rbuckton · 2016-10-13T00:19:37Z

src/compiler/transformers/es2017.ts

+                    return visitAwaitExpression(<AwaitExpression>node);
+
+                case SyntaxKind.MethodDeclaration:
+                    // TypeScript method declarations may be 'async'


Should be ES2017 and not Typescript here

rbuckton · 2016-10-13T00:19:44Z

src/compiler/transformers/es2017.ts

+                    return visitMethodDeclaration(<MethodDeclaration>node);
+
+                case SyntaxKind.FunctionDeclaration:
+                    // TypeScript function declarations may be 'async'


Should be ES2017 and not Typescript here

rbuckton · 2016-10-13T00:33:46Z

src/compiler/types.ts

+        ContainsES2017 = 1 << 5,
+        ES2016 = 1 << 6,
+        ContainsES2016 = 1 << 7,
+        ES6 = 1 << 8,


Please change to ES2015 for consistency.

rbuckton · 2016-10-13T00:34:14Z

src/compiler/types.ts

+        ES2016 = 1 << 6,
+        ContainsES2016 = 1 << 7,
+        ES6 = 1 << 8,
+        ContainsES6 = 1 << 9,


Please change to ContainsES2015 for consistency.

rbuckton · 2016-10-13T00:35:37Z

src/compiler/types.ts

-        AssertES7 = ES7 | ContainsES7,
+        AssertES2017 = ES2017 | ContainsES2017,
+        AssertES2016 = ES2016 | ContainsES2016,
        AssertES6 = ES6 | ContainsES6,


Please change to AssertES2015 for consistency.

rbuckton · 2016-10-13T00:36:27Z

src/compiler/utilities.ts

+                return "lib.es2017.d.ts";
+            case ScriptTarget.ES2016:
+                return "lib.es2016.d.ts";
+            case ScriptTarget.ES6:


Please use ES2015 here for consistency.

rbuckton · 2016-10-13T00:36:56Z

src/harness/harness.ts

+                    return "lib.es2017.d.ts";
+                case ts.ScriptTarget.ES2016:
+                    return "lib.es2016.d.ts";
+                case ts.ScriptTarget.ES6:


Please use ES2015 here for consistency.

andrejbaran · 2016-10-13T11:40:37Z

@mhegazy @rbuckton @sandersn Review the changes pls. I also made the change from ES6 -> ES2015 through out the code so the naming scheme for ES versions is as consistent as possible. Let me know if that's not desired as I realise it's not the purpose of this PR, but since this PR kind of kicked off the consolidation I went for it. Again let me know if you're not ok with it (since it's quite a big change)

sandersn

One minor comment, otherwise looks good. I'm not sure whether we need to duplicate all the async tests to have 2017 versions, but I'll let @rbuckton take a look and decide on that.

sandersn · 2016-10-13T18:06:48Z

src/compiler/types.ts

@@ -3055,7 +3055,9 @@ namespace ts {
        ES5 = 1,
        ES6 = 2,


this is minor, but can you make ES2015=2 and ES6=ES2015 ?

Or maybe even delete ES6 if it's not used any more. I can't remember if this has anything to do with command line parsing, so it might be hard to tell.

The only place where ES6 is used (other than comments) is in command line/compiler options parsing.

removed ES6 completely

mhegazy · 2016-10-14T01:05:09Z

@rbuckton any comments?

rbuckton · 2016-10-14T06:07:44Z

I concur with @sandersn that there are a number of unnecessarily duplicated tests. I need to go through them to see which ones are unneeded. Other than that, 👍

andrejbaran · 2016-10-14T06:52:04Z

@sandersn @rbuckton sorry about that, I thought it would be good to have them for es2017, but didn't find time to dig deeper.

rbuckton

Most of the tests are fine as they also are vetting the emit for --target ES2017. I have marked some tests that are specific to either the checker or our down-level emit for ES6 that do not need to be duplicated.

I can approve this once those changes are made.

rbuckton · 2016-10-14T22:35:05Z

tests/cases/compiler/es2017basicAsync.ts

+    await 0;
+}
+
+const asycnArrowFunc = async (): Promise<void> => {


rbuckton · 2016-10-14T22:37:32Z

tests/cases/conformance/async/es2017/asyncAliasReturnType_es2017.ts

@@ -0,0 +1,6 @@
+// @target: es2017


This test is unnecessary as its only purpose is to verify return type check for async functions, which is already handled by the existing tests. The reason we differ between es5/es6 is that the rules governing whether this is allowed are different when targeting es5/3, but will be the same for es6+

rbuckton · 2016-10-14T22:50:17Z

tests/cases/conformance/async/es2017/asyncClass_es2017.ts

@@ -0,0 +1,4 @@
+// @target: es2017


This test is unnecessary as its only purpose is to verify the checker with respect to async functions, which is already handled by existing tests.

rbuckton · 2016-10-14T22:50:23Z

tests/cases/conformance/async/es2017/asyncConstructor_es2017.ts

@@ -0,0 +1,6 @@
+// @target: es2017


This test is unnecessary as its only purpose is to verify the checker with respect to async functions, which is already handled by existing tests.

rbuckton · 2016-10-14T22:50:37Z

tests/cases/conformance/async/es2017/asyncDeclare_es2017.ts

@@ -0,0 +1,3 @@
+// @target: es2017


This test is unnecessary as its only purpose is to verify the checker with respect to async functions, which is already handled by existing tests.

rbuckton · 2016-10-14T22:51:42Z

tests/cases/conformance/async/es2017/asyncMultiFile_es2017.ts

@@ -0,0 +1,5 @@
+// @target: es2017


This test isn't really needed as it was added only to ensure the __awaiter helper was added to the correct file.

rbuckton · 2016-10-14T22:52:01Z

tests/cases/conformance/async/es2017/asyncQualifiedReturnType_es2017.ts

@@ -0,0 +1,9 @@
+// @target: es2017


This test is unnecessary as its only purpose is to verify the checker with respect to async functions, which is already handled by existing tests.

rbuckton · 2016-10-14T22:52:16Z

tests/cases/conformance/async/es2017/asyncSetter_es2017.ts

@@ -0,0 +1,6 @@
+// @target: es2017


This test is unnecessary as its only purpose is to verify the checker with respect to async functions, which is already handled by existing tests.

rbuckton · 2016-10-14T22:52:50Z

tests/cases/conformance/async/es2017/awaitUnion_es2017.ts

@@ -0,0 +1,14 @@
+// @target: es2017


This test is unnecessary as its only purpose is to verify the checker with respect to async functions, which is already handled by existing tests.

rbuckton · 2016-10-14T22:53:31Z

tests/cases/conformance/async/es2017/functionDeclarations/asyncFunctionDeclaration15_es2017.ts

@@ -0,0 +1,25 @@
+// @target: es2017


This test is unnecessary as its only purpose is to verify the checker with respect to async functions, which is already handled by existing tests.

andrejbaran · 2016-10-14T23:17:54Z

@rbuckton done. Thanks!

mhegazy · 2016-10-14T23:28:01Z

thanks @andrejbaran! waiting on the travis, it has been 🐌 sloooow 🐌 today

rbuckton · 2016-10-15T00:28:53Z

Travis CI looks to be running behind. Once the build passes I'll merge the pull request. Thanks @andrejbaran for the contribution!

rbuckton · 2016-10-15T01:09:10Z

@andrejbaran My appologies, can you take a look at the merge conflict?

andrejbaran · 2016-10-15T06:43:28Z

@rbuckton should be ok now

andrejbaran · 2016-10-17T06:48:08Z

Many thanks guys!

andrejbaran added 2 commits October 6, 2016 00:46

Add ES8/ES2017 target (microsoft#10768)

2c46f9b

ES8/ES2017 target tests

d16e846

msftclas added the cla-already-signed label Oct 5, 2016

mhegazy reviewed Oct 5, 2016

View reviewed changes

mhegazy reviewed Oct 6, 2016

View reviewed changes

andrejbaran added 3 commits October 12, 2016 21:28

Don't use es8. Add es2016 target.

f42c791

Rename es7 to es2016. Update getDefaultLibFileName for new targets.

Move async/await into separate es2017 transformer

5d52c9f

Add es2017 async conformance

1b4c0e3

sandersn requested changes Oct 12, 2016

View reviewed changes

Cleanup comments

e60e97f

andrejbaran changed the title ~~Add ES8/ES2017 target~~ Add ES2017 target Oct 12, 2016

rbuckton suggested changes Oct 13, 2016

View reviewed changes

andrejbaran added 5 commits October 13, 2016 11:58

Make async/await an ES2017 feature only

f0fd77a

Adjust some async conformance baselines

4284a74

Refactor createFunctionExpresson

7df3fda

Remove constant substitution from ES2017 transformer

c5ddf27

Favor use of ES2015 instead of ES6

b871b53

Merge branch 'master' into es2017-target

a500fd9

sandersn approved these changes Oct 13, 2016

View reviewed changes

mhegazy approved these changes Oct 14, 2016

View reviewed changes

Get rid of ES6 completely

86784a5

rbuckton suggested changes Oct 14, 2016

View reviewed changes

Cleanup ES2017 async tests

7352e97

rbuckton approved these changes Oct 14, 2016

View reviewed changes

Merged master into es2017-target

17d60d5

rbuckton merged commit 63160de into microsoft:master Oct 15, 2016

mhegazy mentioned this pull request Oct 15, 2016

Add ES2016 target #5361

Closed

andrejbaran deleted the es2017-target branch October 17, 2016 06:48

mhegazy mentioned this pull request Oct 17, 2016

Include the correct file by default #11685

Merged

mhegazy mentioned this pull request May 29, 2018

Support for --target ES2018 #20463

Closed

3 tasks

microsoft locked and limited conversation to collaborators Jun 19, 2018

Conversation

andrejbaran commented Oct 5, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

msftclas commented Oct 5, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mhegazy commented Oct 6, 2016

Uh oh!

yortus commented Oct 6, 2016

Uh oh!

mhegazy commented Oct 6, 2016

Uh oh!

yortus commented Oct 6, 2016

Uh oh!

andrejbaran commented Oct 6, 2016

Uh oh!

andrejbaran commented Oct 12, 2016

Uh oh!

sandersn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

andrejbaran commented Oct 5, 2016 •

edited

Loading

andrejbaran commented Oct 13, 2016 •

edited

Loading