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

es6 compile #68121

Merged
merged 15 commits into from
Feb 11, 2019
Merged

es6 compile #68121

merged 15 commits into from
Feb 11, 2019

Conversation

jrieken
Copy link
Member

@jrieken jrieken commented Feb 7, 2019

This is for #65372 and makes us emit es6 without dragging in es6-libraries (where possible).

"lib": [
"dom",
"es5",
"es2015.iterable",
Copy link
Member Author

@jrieken jrieken Feb 7, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DanielRosenwasser @mjbvz I cannot get this to work without es2015.iterable. Without it all destructing and for-of is illegal, independent of the downlevelIteration-switch. The downside is that es2015.iterable drags in all of Symbol which we don't want (in the monaco editor and below)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quick conservative workaround:

interface SymbolConstructor {
    readonly iterator: symbol;
}

declare var Symbol: SymbolConstructor;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the Monaco editor also being compiled to ES6 or does it have its own tsconfig.json file?

Copy link
Member

@rbuckton rbuckton Feb 7, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When you use --target es6 we use native destructuring and iteration, which requires Symbol.iterator at runtime, therefore we type-check that the array being destructured/iterated has a [Symbol.iterator]() method that can be used.

If you want to leave this out of the tsconfig.json file, one option is to add a /// <reference lib="es2015.iterable" /> to the files that need iteration.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rbuckton we do have a separate tsconfig.monaco.json which targets ES5.

"es5",
"es2015.iterable",
"webworker.importscripts"
]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't in the schema but exactly what we want 🤓

@jrieken
Copy link
Member Author

jrieken commented Feb 7, 2019

@rbuckton you mentioned that you have a __extends-util that can make a function extend a class. Could you share that with us? Today, all extensions that extend our api class-types are broken.

@jrieken
Copy link
Member Author

jrieken commented Feb 7, 2019

A slightly scary thing that I have encountered is this

let a: any[] = undefined;
let b = [1,2,3]

let c = [...b, ...a]

This snippet above fails to run (correctly) when emitting es6-code but runs when emitting es5-code (which uses concat). That will surely uncover a few unpleasant surprises...

@jrieken jrieken self-assigned this Feb 7, 2019
@jrieken jrieken mentioned this pull request Feb 7, 2019
3 tasks
@mjbvz
Copy link
Contributor

mjbvz commented Feb 7, 2019

@jrieken I think strict null checks may help catch some of these case. With strict mode enabled, this code produces an error:

let a: any[] | undefined = undefined;
let b = [1, 2, 3]

let c = [...b, ...a]

@rbuckton
Copy link
Member

rbuckton commented Feb 7, 2019

@jrieken: Unfortunately, its not __extends that is the problem, but rather how we transform the call to super() in an ES5 class that extends an ES6 class. The issue is that our emit for ES5-downlevel classes uses _super.call, however ES6 classes are spec'd to throw when their internal [[Call]] slot is invoked.

We've discussed changing our down-level emit to support this hybrid approach but that wouldn't help existing packages unless they updated to a version of TypeScript that included that change and recompiled.

Another option would be to use --experimentalDecorators in your API projects and use a decorator to wrap your class in something callable:

function es5ClassCompat(target) {
    function _() { return Reflect.construct(target, arguments, this.constructor); }
    Object.defineProperty(_, "name", Object.getOwnPropertyDescriptor(target, "name"));
    Object.setPrototypeOf(_, target);
    Object.setPrototypeOf(_.prototype, target.prototype);
    return _;
}
...
@es5ClassCompat
class ES6NativeClass {
}

@jrieken
Copy link
Member Author

jrieken commented Feb 8, 2019

re #68121 (comment)

Thanks @rbuckton!

@jrieken jrieken added this to the February 2019 milestone Feb 8, 2019
@Astrantia
Copy link

@jrieken does this improve startup performance?

@joaomoreno joaomoreno deleted the joh/es6 branch February 18, 2019 10:24
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants