First cut of VS Code usage policy

[tsmaeder]

What it does

Draft PR to allow discussion of usage policy for code from VS Code

[colin-grant-work]

Nice work. I've left some copy editing comments and some thoughts.

[colin-grant-work]

This sentence is a little confusing: you can't use code that isn't in monaco-editor-core in Theia, so the question is whether direct reference outside the monaco package is OK.

Suggested change

	So while we don't prohibit the use of code from VS Code (outside of Monaco), we have the following goals:
	So while we don't prohibit the use of code from VS Code (outside of the Monaco package), we have the following goals:

[tsmaeder]

I think "(other than the Monaco editor API)" better captures the intent.

[colin-grant-work]

I believe we currently have some exports from the public API (maybe Range, Position), and those seem relatively unproblematic.

Suggested change

	* Never export a type, function or variable from VS Code from a theia package
	* Never export a type, function or variable from an internal Monaco API from a Theia package.

[tsmaeder]

Some of the Code I VS Code has nothing to do with the Monaco Editor, see for example the quick-input component I mentioned in the intro . Even if we do use some types now, I believe we should not add new such cases.

[colin-grant-work]

I feel like this is the whole gray area, so it isn't very clear guidance. For one thing, I'd like to discourage copying in general. By definition, it's redundant, and it means that if any bugs or vulnerabilities are found and fixed in the copied code, we won't get them, and likely won't even know about them, unless someone reports them separately for our repo. It's also easy to copy without realizing that the same code has already been copied - we had at least two copies (but I think three) each of a couple of VSCode utilities that had been near the root of the plugin-ext package, in one corner of plugin-ext, and also in core - and now there's one copy in core, which is already mostly redundant given that most applications will also have monaco in them anyway. On another hand, if something is very easy to copy from VSCode, then the value of the import isn't very great, other than avoiding redundancy - the reason to import something is because that's a better bet than reimplementing / copying it. Every decision about whether to copy, import, or reimplement is a tradeoff of development time, ease of maintenance, and functionality, and evaluating where to come down in a particular case is not easy.

[tsmaeder]

The idea is to not increase coupling to the VS Code internals. We're in a relatively good place now, but we were not in the past. Updating to a new VS version should be a trivial task we can do frequently without undue burden. If we use large parts of unstable VS code (unstable in the sense that the API changes), we will have to update our usages each time we update to a new VS Code version. The "escape hatch" from this coupling (if the code in VS goes into a direction we don't want to follow). Therefore, "I could easily copy this over if I need to" seems to be a good heuristic to me. .

[tsmaeder]

If we had copies of utilities in the code we wrote ourselves, why would we not have copies of utilities when we import them from VS? The problem seems to be that people are not aware a thing already exists. I would expect folks to be more familiar with the Theia code base than with the VS codebase, thus creating less copies.

[colin-grant-work]

deeply coupled with other parts of VS Code which we don't want to import.

I don't know if this really carries weight. We're starting with a tree-shaken (gently - admittedly we could be more aggressive), heavily reduced version of VSCode in monaco-editor-core, and we import the public API (as well some internal stuff), which is the basis for the tree shaking. That means that anyone who depends on monaco-editor-core through Theia will be getting at least every file in that package - if they tree-shake the application more thoroughly, it's possible they don't get every line of code, but they get at least every file - so there aren't really any big parts of VSCode that are in the package the user isn't already importing.

Given that, I think the only real question is stability / maintenance. I absolutely agree that internal types from Monaco shouldn't be exported unadapted to adopters, but I think the only other real criterion to consider is the estimated cost of maintaining whatever API (adapted, reimplemented, or copied) that we do present to the world.

[tsmaeder]

Dead code elimination is fine as far as it goes, but even just importing a simple feature (again: quick-input), might use large swathes of code in it's implementation. We just don't know just by looking at the API. We do know with the Monaco editor, because the VS Code team themselves want to make sure it has relatively compact dependencies, since the publish it as a distinct package.

[MatthewKhouzam]

I like the clarifications here. It makes it clear how we are to work.

[vince-fugnitto]

I believe the ECA should pass if you squash and force-push, it's likely the no-reply email that is causing problems:

From: =?UTF-8?q?Thomas=20M=C3=A4der?= <[email protected]>

[vince-fugnitto]

minor:

Suggested change

Since its inception, Theia has used the "Monaco" editor component from VS Code. With the recent move to using ECMAScript modules, consuming code from the VS Code project has become much easier and safer. But while reusing code saves us work, there is also a down-side to it. Monaco has a relatively stable external API because Microsoft also releases it as a stand-alone editor component. But other parts of the code base may change more frequently and in unexpected ways. We always use the same version of all modules making up VS Code. So when we update VS Code, often to provide a new feature in Monaco to our adopters, we will have to deal with all the API changes at that same time. As an example: Theia used the quick-input component from VS Code directly to implement it's own quick-input component. Because the component was not encapsulated in any way, the updating Monaco to a new version became difficult and time-consuming.

Since its inception, Theia has used the "Monaco" editor component from VS Code. With the recent move to using ECMAScript modules, consuming code from the VS Code project has become much easier and safer. But while reusing code saves us work, there is also a downside to it. Monaco has a relatively stable external API because Microsoft also releases it as a standalone editor component. But other parts of the code base may change more frequently and in unexpected ways. We always use the same version of all modules making up VS Code. So when we update VS Code, often to provide a new feature in Monaco to our adopters, we will have to deal with all the API changes at that same time. As an example: Theia used the quick-input component from VS Code directly to implement it's own quick-input component. Because the component was not encapsulated in any way, the updating Monaco to a new version became difficult and time-consuming.

[tsmaeder]

"downside" yes, but "stand-alone" ist still more common according to the internet.

[vince-fugnitto]

minor:

Suggested change

The first rule shields our adopters from having to change their code in response to updating Monaco. If they cannot see an object or type, they cannot rely on its existence or API. Note that this includes functions, supertype relationships or parameter and return types. If you need to export functionality, export an interface in Theia and import that interface using imported code from VS Code. This way, we can build adapters to shield against API changes. The rule also prevents spreading dependencies on VS Code in our code without our being aware of them. While it's not technically possible to enforce non-export of stuff in our current build system, we should make sure we're not exporting tainted code through `index.ts` or similar mechnisms. At the very least, we should not require package users to rely on VS Code stuff.

The first rule shields our adopters from having to change their code in response to updating Monaco. If they cannot see an object or type, they cannot rely on its existence or API. Note that this includes functions, supertype relationships or parameter and return types. If you need to export functionality, export an interface in Theia and import that interface using imported code from VS Code. This way, we can build adapters to shield against API changes. The rule also prevents spreading dependencies on VS Code in our code without our being aware of them. While it's not technically possible to enforce non-export of stuff in our current build system, we should make sure we're not exporting tainted code through `index.ts` or similar mechanisms. At the very least, we should not require package users to rely on VS Code stuff.

[vince-fugnitto]

The string import from @theia/monaco-editor-core/esm/vs does not actually produce results in the codebase as is, we can simplify to:

Suggested change

	Tip: you can find locations where VS code is used searching for `import from @theia/monaco-editor-core/esm/vs`
	Tip: you can find locations where VS code is used by searching for import statements from `@theia/monaco-editor-core/esm/vs`.