-
Notifications
You must be signed in to change notification settings - Fork 30k
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
es6 compile #68121
Conversation
src/tsconfig.json
Outdated
"lib": [ | ||
"dom", | ||
"es5", | ||
"es2015.iterable", |
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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;
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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" | ||
] |
There was a problem hiding this comment.
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 🤓
@rbuckton you mentioned that you have a |
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 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] |
@jrieken: Unfortunately, its not 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 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 {
} |
Thanks @rbuckton! |
@jrieken does this improve startup performance? |
This is for #65372 and makes us emit es6 without dragging in es6-libraries (where possible).