Provides @eclipse-che/plugin namespace to VS Code extensions

[benoitf]

Is your enhancement related to a problem? Please describe.

Today, VS Code extensions don't have access to @eclipse-che/plugin namespace and if they're linking to this module it may not be deployed easily on VS Code.

Describe the solution you'd like

Export @eclipse-che/plugin namespace as a VS Code extension export through a Theia plug-in (that has access to the namespace for example)
https://code.visualstudio.com/api/references/vscode-api#extensions

VS Code extensions wanting to use Che features could then do:

const eclipseChePluginExport = vscode.extensions.getExtension('@eclipse-che/plugin');
if (eclipseChePluginExport) {
   eclipseChePluginExport.workspace.getCurrentWorkspace()
}

Describe alternatives you've considered

N/A

Additional context

Leverage VS Code extension paradigm

[flacatus]

I will set P1 feel free to change

[fbricon]

So I've tried this in a vscode extension:

import { IdManager } from '@redhat-developer/vscode-redhat-telemetry/lib';
import { UUID } from '@redhat-developer/vscode-redhat-telemetry/lib/utils/uuid';

export class CheIdManager implements IdManager {
    async getRedHatUUID(): Promise<string> {
        try {
            const che = require('@eclipse-che/plugin');
            const user = await che.user.getCurrentUser();
            return user.id;
        } catch (error) {
            console.log(`Failed to get user id from Che: ${error}`);
            //fall back to generating a random UUID
            return UUID.getRedHatUUID();
        }
    }
}

But, when @benoitf tested it in Che, he got :

Failed to get user id from Che: Cannot find module @eclipse-che/plugin

This is critical for us to get proper telemetry from vscode extensions running in Che

[tsmaeder]

I think this is a duplicate of #17609. Using Che API in remote plugin hosts.

[benoitf]

It's not related at all.

[tsmaeder]

Well: using Che api in remote plugin hosts does not work, that's why we get the "actor chetelemetry not found". That would explain why it's not working for @fbricon. Do you not agree?

[tsmaeder]

It would be easy to test: if we run it in the theia container and it works, my hypothesis is correct.

[benoitf]

vscode-commons was loaded in the theia container as it's a pure nodejs app and it failed (We tested with vscode-commons only). So your hypothesis is not correct.

[tsmaeder]

I checked the extension vsix @fbricon provided and it has a problem with packaging: it never calls require("@eclipse-che/plugin"). I'm still pretty convinced we don't need this if the vsix is properly built.

[tsmaeder]

@benoitf could you share your analysis of the problem? What's going wrong that prevents the plugin api from being loaded? I was able to debug successfully through a import cheApI = require('@eclipse-che/plugin') when fixing the webpack config in https://github.com/redhat-developer/vscode-commons

[benoitf]

Analysis of the problem was pretty simple.
VS Code offers an ability to reuse code from 3rd party extensions in a pretty simple way that all developers are familiar with.
So sticking to this model allow developers to re-use Che-Theia stuff without deep knowledge of Che-Theia or having to tune their codebase, without using try/catch, using require keyword, etc.

It's Che-Theia that needs to fit VS Code extension model, not the opposite.

[tsmaeder]

@benoitf So there is no actual bug being solved here. require("@eclipse-che/plugin") would work if the plugin was properly packaged, correct? But plugins will have to use your extensions and ask for it's API, so they will have to "tune" their code base anyway. Having to treat "vscode" as an external module is something plugin writers already do, so no new concepts required here. Also, you're returning the exact same API as the Che API, so clients will have to know how to use it just the same.

About the try-catch: it's the idiomatic way you can test for the presence of a module in node. If you don't want to load the module, you can just require.resolve().

Sorry, but I don't see a problem being fixed and I don't see an additional capability being added by this work. If code doesn't solve a problem, it should not exist. Again, I'm truly open to be convinced if you can point me to any scenario that is made possible or even easier by this effort.

[azatsarynnyy]

So, the argument here is about two different approaches.

1. More generic way.
   require + try-catch + webpack config
   It suites well for any library and requires some knowledge of how all these things work together.
   Also, it requires some time to configure everything properly.

2. Approach that is provided for the VS Code extensions only.
   vscode.extensions.getExtension
   It's VS Code's official way of consuming other extension's API.
   AFAIK, it's the easiest way to check if some extensions is present in the environment and call the needed functionality.

I suppose the time is needed for applying the 1'st approach would be several hours for a not very experienced developer.
Applying the 2'nd approach would require just several minutes. It's just my roughly guessing.

So, why should a developer bother with require/try-catch/webpack/etc?
Isn't it obvious what is more convenient and fast approach? I mean eclipse-che/che-theia#994

It would be great to hear @fbricon's opinion on that. As he is the first consumer of this feature as I understand.

[tsmaeder]

Applying the 2'nd approach would require just several minutes

No it's not. Node.js require is not some esoteric API no-one ever uses. And if you use webpack, you'll have seen that you need to declare API packages as external.

I object to this change because it duplicates existing functionality. Also: you'll get an any value from "getExtension()", whereas if you require the API, the typescript compiler will check your API calls.

[tsmaeder]

Also, I'm not convinced that clients would even prefer the "getExtension()" way if the can import a typed API instead of an any value.

[azatsarynnyy]

I don't think it worth spending so much time discussing such a minor thing.
If require solves the initial problem - that's great! What we're discussing then? Just close the issue.
If it doesn't work for the given case - then what's the problem with merging eclipse-che/che-theia#994?

[tsmaeder]

I've since talked to @fbricon and he kinda prefers not to have to add a line to his webpack config. So I guess one could construe that as a community request. I still think it's just code duplication, but I'll remove the block from the PR.
This could have been much easier if there had been more of a motivation for the issue (I mistakenly thought this was fixing something that doesn't work, due to #18837 (comment)). Next time, just give me a call and clear it up.

[fbricon]

@azatsarynnyy summarized it best. I can work with both solutions. #1 didn't work for me initially though, #2 worked immediately, so I'm totally OK with it

[benoitf]

merged in 7.27