Make vscode/theia APIs contributed instead of built-in #8180

tsmaeder · 2020-07-16T08:21:43Z

Signed-off-by: Thomas Mäder tmader@redhat.com

What it does

This a proof-of-concept PR for for issue Make "theia" and "vscode" contributed API's #8142

The idea is to not have any built-int API namespaces like 'theia' and 'vscode', but instead to only have a single mechanism for contributing API's. This PR is to show feasibility and to invite discussion. I have tested it as far as running the typescript-language-features built-in extension in a workspace containing the theia source. Plugins using the theia namespace and front-end plugins have not been tested so far.
The long-term idea would be to separate packages/plugin-ext into packages/plugin-ext and packages/plugin-ext-theia. Doing so should separate the mechanics of running plugins from the concrete API implementation. This will make the code base easier to understand and maintain.

How to test

This should not change anything in the behaviour of theia. Any front-end or back-end plugin should work just like before.

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

akosyakov · 2020-07-28T09:01:19Z

packages/plugin-ext-vscode/src/browser/vscode-main-plugin-api-provider.ts

+
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    async computeInitParameters?(rpc: RPCProtocol, container: interfaces.Container): Promise<TheiaAPIInitParameters> {
+        const theiaMainPluginAPIProvider: TheiaMainPluginAPIProvider = container.get(TheiaMainPluginAPIProvider);


passing Container around is anti-pattern, one should use @inject

Note that "initialize" does the same and unfortunately, using "@Inject" leads to errors at runtime. I have no idea why, though.

What's the error?

Some inversify error, but since this was > 1 year ago, I can't really remember precisely.

akosyakov · 2020-07-28T09:03:45Z

Till VS Code extensions are working and there are not much over engineering to support one additional namespace is fine with me.

akosyakov · 2020-07-28T09:30:36Z

packages/plugin-ext/src/hosted/browser/hosted-plugin.ts

+                };
+                const mainProvider = mainApiProviders.get(api.id);
+                if (mainProvider && mainProvider.computeInitParameters) {
+                    result.initParameters = await mainProvider.computeInitParameters(rpc, this.container);


we should check whether we disconnected after any async call and exit with undefined as it was before

you mean like

if (toDisconnect.disposed) { return undefined; } ``

benoitf · 2020-07-28T11:39:14Z

Plugins using the theia namespace and front-end plugins have not been tested so far.

should be tested with theia namespace but also che namespace for my approval

overall is ok for me, just remove the che occurence from the code

The long-term idea would be to separate packages/plugin-ext into packages/plugin-ext and packages/plugin-ext-theia. Doing so should separate the mechanics of running plugins from the concrete API implementation. This will make the code base easier to understand and maintain.

I think long term is to have vscode API implemented in a single place and theia namespace being implemented only for stuff that does not fit within vscode namespace. Not sure if this PR helps.

tsmaeder · 2020-07-28T12:10:29Z

should be tested with theia namespace but also che namespace for my approval

I agree for the theia namespace, but this PR should not change anything at all for the 'che' namespace. All namespaces being created equal is the point of this PR.

I think long term is to have vscode API implemented in a single place and theia namespace being implemented only for stuff that does not fit within vscode namespace. Not sure if this PR helps.

Currently we can only use either the theia or the vscode namespace, but not both for the same plugin. This PR enables using both namespaces in parallel. That is a prerequisite for removing the vscode duplication from the 'theia' namespace.

tsmaeder · 2020-07-28T12:11:50Z

Till VS Code extensions are working and there are not much over engineering to support one additional namespace is fine with me.

Sorry, but I have no idea what you're trying to say here 🤷

Artnoc1 · 2020-08-03T07:33:36Z

Signed-off-by: Thomas Mäder tmader@redhat.com

What it does

This a proof-of-concept PR for for issue Make "theia" and "vscode" contributed API's #8142

The idea is to not have any built-int API namespaces like 'theia' and 'vscode', but instead to only have a single mechanism for contributing API's. This PR is to show feasibility and to invite discussion. I have tested it as far as running the typescript-language-features built-in extension in a workspace containing the theia source. Plugins using the theia namespace and front-end plugins have not been tested so far.
The long-term idea would be to separate packages/plugin-ext into packages/plugin-ext and packages/plugin-ext-theia. Doing so should separate the mechanics of running plugins from the concrete API implementation. This will make the code base easier to understand and maintain.

How to test

This should not change anything in the behaviour of theia. Any front-end or back-end plugin should work just like before.

Review checklist
* [ ]  as an author, I have thoroughly tested my changes and carefully followed [the review guidelines](https://github.com/theia-ide/theia/blob/master/doc/pull-requests.md#requesting-a-review)
Reminder for reviewers
* as a reviewer, I agree to behave in accordance with [the review guidelines](https://github.com/theia-ide/theia/blob/master/doc/pull-requests.md#reviewing)

tsmaeder · 2021-09-08T08:06:05Z

@arekzaluski, @paul-marechal an feedback welcome.

Signed-off-by: Thomas Mäder <tmader@redhat.com>

JonasHelming · 2022-02-09T10:27:08Z

@tsmaeder : Decision needed

tsmaeder · 2022-02-09T13:53:40Z

I don't think anyone is going to merge this one any time soon. I've thought about it some more recently, and I think we'd have to solve a couple of problems with instantiating main/ext services if the vscode and theia namespaces become independent. Still a good idea, though...

tsmaeder requested review from benoitf and akosyakov July 16, 2020 08:21

akosyakov added the plug-in system issues related to the plug-in system label Jul 28, 2020

akosyakov reviewed Jul 28, 2020

View reviewed changes

tsmaeder mentioned this pull request Dec 2, 2020

allow to consume only subset of VS Code functionality #7754

Open

paul-marechal mentioned this pull request Apr 6, 2021

PoC - cli: filter Theia extensions #9309

Closed

1 task

tsmaeder mentioned this pull request Aug 12, 2021

Added detailed description about extension types eclipse-theia/theia-website#186

Merged

tsmaeder force-pushed the 8141_contribute_APIs_2 branch from 10deabf to 01e01f1 Compare September 8, 2021 07:50

tsmaeder requested review from paul-marechal and removed request for benoitf September 8, 2021 08:02

tsmaeder marked this pull request as ready for review September 8, 2021 08:06

Make vscode/theia APIs contributed instead of built-in

827430f

Signed-off-by: Thomas Mäder <tmader@redhat.com>

tsmaeder force-pushed the 8141_contribute_APIs_2 branch from 01e01f1 to 827430f Compare November 23, 2021 09:04

JonasHelming added the decision_needed issues that require decisions on ways forward (ex: if they should be updated or closed) label Feb 9, 2022

JonasHelming assigned tsmaeder Feb 9, 2022

tsmaeder closed this Feb 9, 2022

tsmaeder mentioned this pull request Oct 2, 2023

Need ability to add custom plugin API namespace that's directly connected to a backend implementation #12946

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make vscode/theia APIs contributed instead of built-in #8180

Make vscode/theia APIs contributed instead of built-in #8180

tsmaeder commented Jul 16, 2020

akosyakov Jul 28, 2020

tsmaeder Jul 28, 2020

paul-marechal Sep 8, 2021

tsmaeder Sep 8, 2021

akosyakov commented Jul 28, 2020

akosyakov Jul 28, 2020

tsmaeder Oct 29, 2020

benoitf commented Jul 28, 2020 •

edited

Loading

tsmaeder commented Jul 28, 2020 •

edited

Loading

tsmaeder commented Jul 28, 2020

Artnoc1 commented Aug 3, 2020

What it does

How to test

Review checklist

Reminder for reviewers

tsmaeder commented Sep 8, 2021

JonasHelming commented Feb 9, 2022

tsmaeder commented Feb 9, 2022

Make vscode/theia APIs contributed instead of built-in #8180

Make vscode/theia APIs contributed instead of built-in #8180

Conversation

tsmaeder commented Jul 16, 2020

What it does

How to test

Review checklist

Reminder for reviewers

akosyakov Jul 28, 2020

Choose a reason for hiding this comment

tsmaeder Jul 28, 2020

Choose a reason for hiding this comment

paul-marechal Sep 8, 2021

Choose a reason for hiding this comment

tsmaeder Sep 8, 2021

Choose a reason for hiding this comment

akosyakov commented Jul 28, 2020

akosyakov Jul 28, 2020

Choose a reason for hiding this comment

tsmaeder Oct 29, 2020

Choose a reason for hiding this comment

benoitf commented Jul 28, 2020 • edited Loading

tsmaeder commented Jul 28, 2020 • edited Loading

tsmaeder commented Jul 28, 2020

Artnoc1 commented Aug 3, 2020

What it does

How to test

Review checklist

Reminder for reviewers

tsmaeder commented Sep 8, 2021

JonasHelming commented Feb 9, 2022

tsmaeder commented Feb 9, 2022

benoitf commented Jul 28, 2020 •

edited

Loading

tsmaeder commented Jul 28, 2020 •

edited

Loading