Transpile theia to ES2017

[sgraband]

What it does

This is is a PR for transpiling Theia to ES2017. To continue supporting es5 based VSCode extensions, the @es5ClassCompat tag was used.

The tag was also applied to be used with Theia es5 plugins.

We would like to gather feedback while tackling the remaining issues, so feel free to comment on this PR!

Open issues:

• The @es5ClassCompat tag is not working for all of the classes in types-impl.ts.
  The tag is failing on Location and DiagnosticRelatedInformation because of an Cannot access Location before its initialization error
  Applying the tag to CodeLens, TreeItem, ProgressExecution, ShellExecution and DebugAdapterExecutable will result in a Can't resolve @theia/plugin ERROR.
• Test support for es5 Theia plugins.
• In targeting es6 as a base (without transpiling to es5) #5902 it was mentioned that there could be issues with phosphorjs and inversifyjs, however we could not find any regressions while testing. This may need some further research.

Contributed on behalf of STMicroelectronics
Signed-off-by: Simon Graband sgraband@eclipsesource.com

How to test

To test the support for es5 VSCode extensions, you can try out one of the extensions described here: microsoft/vscode#68698.

Review checklist

• as an author, I have thoroughly tested my changes and carefully followed the review guidelines

Reminder for reviewers

• as a reviewer, I agree to behave in accordance with the review guidelines

[paul-marechal]

@sgraband feel free to use the following commit to address the issues you were mentioning with es5ClassCompat.

The Cannot access Location before its initialization error was due to how code generation works for decorators and bad ordering.

The Can't resolve @theia/plugin error was due to TypeScript thinking it needed to emit a require call when it did not.

[paul-marechal]

Also, what about the following configuration?

theia/dev-packages/application-manager/src/generator/webpack-generator.ts

Lines 177 to 196 in 61200ef

	{
	test: /\\.js$/,
	// include only es6 dependencies to transpile them to es5 classes
	include: /vscode-ws-jsonrpc|vscode-jsonrpc|vscode-languageserver-protocol|vscode-languageserver-types/,
	use: {
	loader: 'babel-loader',
	options: {
	presets: ['@babel/preset-env'],
	plugins: [
	// reuse runtime babel lib instead of generating it in each js file
	'@babel/plugin-transform-runtime',
	// ensure that classes are transpiled
	'@babel/plugin-transform-classes'
	],
	// see https://github.com/babel/babel/issues/8900#issuecomment-431240426
	sourceType: 'unambiguous',
	cacheDirectory: true
	}
	}
	}

It transpiles some ES6 dependencies down to ES5, but those should no longer be required right?

[sgraband]

I tested the support for es5 theia plugins. It is working as intended. I noticed that it is enough to apply the @es5ClassCompat tag in types-impl.ts, so i removed it from theia.d.ts.

@paul-marechal could you do a final review please? I will set this PR as ready for review.

[paul-marechal]

I noticed a couple of classes not decorated by es5ClassCompat, is this intended? When should we use the decorator?

theia/packages/plugin-ext/src/plugin/types-impl.ts

Lines 1478 to 1479 in bede031

	export class FileDecoration {

theia/packages/plugin-ext/src/plugin/types-impl.ts

Lines 1577 to 1578 in bede031

	export class ProgressOptions {

theia/packages/plugin-ext/src/plugin/types-impl.ts

Lines 1599 to 1600 in bede031

	export class Progress<T> {

theia/packages/plugin-ext/src/plugin/types-impl.ts

Lines 1778 to 1779 in bede031

	export class CustomExecution {

theia/packages/plugin-ext/src/plugin/types-impl.ts

Lines 2301 to 2302 in bede031

	export class CallHierarchyItem {

theia/packages/plugin-ext/src/plugin/types-impl.ts

Lines 2336 to 2337 in bede031

	export class CallHierarchyIncomingCall {

theia/packages/plugin-ext/src/plugin/types-impl.ts

Lines 2347 to 2348 in bede031

	export class CallHierarchyOutgoingCall {

theia/packages/plugin-ext/src/plugin/types-impl.ts

Lines 2376 to 2377 in bede031

	export class SemanticTokensLegend {

theia/packages/plugin-ext/src/plugin/types-impl.ts

Lines 2389 to 2390 in bede031

	}
	export class SemanticTokensBuilder {

theia/packages/plugin-ext/src/plugin/types-impl.ts

Lines 2568 to 2569 in bede031

	export class SemanticTokens {

theia/packages/plugin-ext/src/plugin/types-impl.ts

Lines 2578 to 2579 in bede031

	export class SemanticTokensEdit {

theia/packages/plugin-ext/src/plugin/types-impl.ts

Lines 2590 to 2591 in bede031

	export class SemanticTokensEdits {

[sgraband]

Good Catch! They were missing because these classes are not part of the vscode API, which i looked at first. However, it is probably a good idea to decorate all the classes to ensure compatibility with plugins, that use the theia specific classes. I pushed my changes (which should also solve the build issues).

[sgraband]

Rebased to master and added entry to changelog.

[paul-marechal]

CI is red, please fix :)

[paul-marechal]

CI is still failing. Feel free to ask a committer that would be in your timezone to approve and run the CI for you in order to make the feedback loop shorter, or run the tests locally before pushing.

[sgraband]

I fixed the error in the CI. The error that is currently happening seems to be unrelated to my changes, as the master build has the same error.

[paul-marechal]

I've been looking at the generated ES6 code and it looks mostly good!

I am only slightly bothered by the output for async functions. When targeting ES6, TypeScript will output things like:

// source
async function test() {
    await 1
}

// generated ES6 (ES2015)
"use strict";
var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, generator) {
    function adopt(value) { return value instanceof P ? value : new P(function (resolve) { resolve(value); }); }
    return new (P || (P = Promise))(function (resolve, reject) {
        function fulfilled(value) { try { step(generator.next(value)); } catch (e) { reject(e); } }
        function rejected(value) { try { step(generator["throw"](value)); } catch (e) { reject(e); } }
        function step(result) { result.done ? resolve(result.value) : adopt(result.value).then(fulfilled, rejected); }
        step((generator = generator.apply(thisArg, _arguments || [])).next());
    });
};
function test() {
    return __awaiter(this, void 0, void 0, function* () {
        yield 1;
    });
}

We currently target Node 12.14.1 which supports at least ES2017 and native async/await notation. Since we share the same configuration between backend and frontend it means outputting native async/await would have to be supported in browsers to.

Using ES2017 gives:

// generated ES2017
"use-strict";
async function test() {
    await 1;
}

Thankfully it looks like most browsers/users support this: https://caniuse.com/async-functions

We also already expect the browser to support WebAssembly which is supposedly "newer" than async/await support:

• async/await browser compatibility: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/async_function#browser_compatibility
• WebAssembly browser compatibility: https://developer.mozilla.org/en-US/docs/WebAssembly#browser_compatibility

I think we should at least target ES2017. The biggest breaking change happened when new style classes got introduced. Since then most things look backward-compatible.

[paul-marechal]

LGTM! @eclipse-theia/core?

[msujew]

LGTM as well. Couldn't notice any regression on browser or electron (Ubuntu+Windows).

[msujew]

Nitpick: As 1.15 was released last month, this should probably be removed.

[tsmaeder]

I'm just wondering what the impact of the change on adopters will be. Will they have to update their build story as well? Maybe a good topic for the community call.

[sgraband]

@tsmaeder we tested this with a theia template from the yeoman generator and there is no need for adopters to change their build story. If for some reason they still need an es5 codebase they should be able to transpile back to es5 using webpack. I am not quite sure if this is really a use case and if we should add this to the breaking changes section in the changelog?

[paul-marechal]

if we should add this to the breaking changes section in the changelog?

Yes. As I mentioned in the dev meeting last week, we use some es5ClassCompat decorator for plugin classes but not for Theia API classes. So we should at least mention that we moved from ES5 to ES2017 as a (potential) breaking change.

[paul-marechal]

I think this should be placed under 1.17.0 breaking changes now.

[tsmaeder]

Now that we have a migration guide, how about adding a section on what adopters will have to do?

[sgraband]

I placed the changelog entry in the 1.17.0 breaking changes. However, i am not sure what you mean with the migration guide? Should i add more information to the changelog? If yes, what information would you expect?

[vince-fugnitto]

@tsmaeder @sgraband the migration guide is not yet merged, we can update it after if necessary:

• documentation: include migration guide #9817

[paul-marechal]

@sgraband you forgot to remove the old entry under 1.16.0

[sgraband]

I removed the old changelog entry.

[sgraband]

I added an entry to the migration guide and rebased to master.

[vince-fugnitto]

The code changes look good to me, I did not notice any regressions when testing (verified inversify and phosphor as well) 👍

[sgraband]

Is there anything else to do? Or can this be merged?

[paul-marechal]

Let's bring that up during the dev meeting tomorrow and merge after that if there's no objections.

[paul-marechal]

@sgraband can you rebase? If CI is green after that and you don't notice anything while running then I think it is fine to merge!

[sgraband]

I rebased and could not find any regressions. Could someone approve the workflow?