-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Implement "headless plugins" in a new plugin host outside of frontend connections #13138
Conversation
e895af9
to
3ab68f1
Compare
I agree. |
We definitely would not want that given how many Theia front-end things |
Currently the To factor out the code from |
That's a good point. It's unlikely someone intending to write a CLI application would come to Theia. With the introduction of this new plugins stack, I suppose it makes the scenario more likely, though. I.e., if I wanted to write a CLI application in TypeScript and support third parties extending the application using a robust plugin mechanism that firewalls the main application from the plugins...then maybe??? But it's a stretch, I'll admit. I don't think the likelihood warrants further work at this point. We can cross that bridge if and when we get there. A (slightly?) more likely scenario is a Theia app that developer that wants to allow extensions of his app only via this new type of plugin and not allow customization at the GUI level. There again...let's cross the bridge when we need to. |
3ab68f1
to
b79be18
Compare
Commit b79be18 updates the PR to
The PR description is updated accordingly. |
I don't think "headless" is a good term to describe this kind of plugin because that is not the relevant distinction: I would argue most plugins would hook into some kind of UI. The distinction is scope: the back end plugins are shared among all front-ends. And "back end" is the right term here, since they are scoped to what we call the "back end" (as opposed to the Electron main process, for example). As with dependency injection, there is really no conceptual reason to not consider runtime extensibility for all parts of our system, including the Electron main process. |
I've looked at the PR on a very high level, and here are a couple of suggestions.
|
I respectfully disagree. I think it is precisely the relevant distinction. There are currently two types of plugins in Theia (and VS Code). One runs in the backend, the other in the frontend, but both are wired to the frontend. They drive functionality in the UI (the "head"). The new type of plugin this PR introduces not only lives in the backend but interacts only with the backend (of Theia extensions). Thus "headless" actually seems like a very suitable term. As one of the existing types of plugins today runs in the backend, I think calling this new thing a "backend plugin" would be pretty confusing.
The Electron main process is really two things running side-by-side in the same process: Node.js and the Chromium browser. I.e., it is both the frontend and the backend. Thus I'm not really understanding your point here. |
The "back end" is a process running an express server that is separate from the Electron main process. |
Oh wow. Just when I thought I was understanding Theia/electron/etc pretty well... I don't know how I missed that process when I've looked in process explorer. |
792d97d
to
15f3886
Compare
Commit 99b5002 removes the It did require refactoring a few dependencies on concrete Ext implementations in the common plugin manager code to use instead internal interfaces that could be stubbed with no-ops for the headless plugin manager. The GotD example plugin is updated to not attempt to use the API that no longer is available. Otherwise, everything seems to run as before (headless and VS Code plugins). |
af3f9ca
to
99b5002
Compare
I wonder if we shouldn't be using |
For whatever reason, |
It is exposed in the electron app's stdout. It's yet another unfortunate discrepancy that is likely to cause confusion (another is the "extension" vs "plugin" naming thing). I believe most plugin development for Theia apps will be of the type VS Code extensions and headless plugins. I'm told people aren't really writing |
Commit ccc1181 adds support for the "headless" entry point in a plugin that otherwise is an ordinary VS Code plugin. And because of this the expected start/stop lifecycle function names are changed to
|
a551b36
to
ccc1181
Compare
Thank you! |
@cdamus I'm still working with your initial commit, as I had to rebase your changes onto Theia 1.43.0 and that was a bit tricky. Bringing in any further changes might take a little time. So far, you haven't changed anything fundamental in the subsequent commits so I'll keep trucking as-is for now. It appears that the headless plugin isn't activated unless I have the following in the plugin's package.json
which is unexpected. My understanding is that there is no support yet for activation events, so I assume the plugin should be loaded unconditionally at this point. |
Or maybe it's not that there is no support for activation events and that there just aren't any activation events to trigger off of. So you have to basically request to be loaded unconditionally with "*". If that's the case, then we'd for sure want to use a unique package.json element specifically for headless plugin--maybe |
I had thought about this. There are no activation events specified by the framework, but I expect that applications will naturally find their own suite of events according to what their extension points are that headless plugins will supply. So I thought that applications could read the Does that make sense? BTW, is there a problem with adding application-specific activation events that VS Code doesn't understand to the |
Yes. It does.
That's all fine and well when plugins specify actual activation events. We're sort of counting on there being no name collision on the activation events between the two types of plugins. Not ideal, but that will probably be fine. The real issue comes when one type of plugin wants to be activated by default ("*"). Under the current design/implementation, a "*" dictates the behavior for both types of plugins, which we really don't want. I might have a plugin package with a very lightweight headless plugin but a beast of a VS Code extension. If I need the headless plugin to be loaded by default, that means an unnecessary heavy penalty on the frontend. One approach is to have a distinct string to represent "*" for headless. But that starts looking ugly. With the approach I recommended ( |
f100932
to
dab5684
Compare
Commit dab5684 rebases the branch onto the latest
|
Discussed off-line, it's clear that, in fact, we should be able to accomplish this using the plugin module loading already in place. As we need to load a module to access the start/stop functions and API factory of the plugin already, this can be expanded to access an Inversify container-module to configure our host's DI container. |
3459bf7
to
3c0fc54
Compare
packages/plugin-ext/src/main/node/handlers/plugin-theia-directory-handler.ts
Outdated
Show resolved
Hide resolved
packages/plugin-ext-headless/src/hosted/node/headless-hosted-plugin.ts
Outdated
Show resolved
Hide resolved
packages/plugin-ext-headless/src/plugin/headless-plugin-manager.ts
Outdated
Show resolved
Hide resolved
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.
Hi Christian, thank you very, very much for that change! I had a look at the code and will do some runtime testing a little bit later. From a first glance everything looks good and I believe this feature will add a lot of value to Theia, especially with the introduction of Inversify on the extension host side. My comments are mostly regarding understanding and extensibility. As soon as everything is reviewed and agreed upon, we should consider whether the documentation properly explains the possibilities and pit falls. In any case, great work!
packages/plugin-ext-headless/src/main/node/plugin-ext-headless-main-module.ts
Outdated
Show resolved
Hide resolved
3c0fc54
to
8eaf8fe
Compare
Thanks @jfaltermeier and @martin-fleck-at for your reviews! I think I have addressed most, if not all, of your comments with commit 5e877b8. Overall I think this really helped to clean up and rationalize the code! |
8eaf8fe
to
5e877b8
Compare
@cdamus Not sure if I'm confused, but at some point, this PR had a number of commits that reflected the evolving nature of the change. Today I see just one commit. Did you collapse all the commits into one and force push your branch? |
Yes. There is a lot of concurrent activity in the keeping-up-with-VSCode-API sphere that requires occasional conflict resolution, and it's not feasible to maintain multiple commits because every commit requires manual conflict resolution every time. So I have to keep this squashed. |
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.
Thank you very much for the detailed explanation and the updated PR. Everything looks good to me now from a code and runtime perspective. Great work! Since this is such a large PR, we'll ask another reviewer to have a look before merging it.
packages/plugin-ext-headless/src/hosted/node/plugin-host-headless-module.ts
Outdated
Show resolved
Hide resolved
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 looks alright to me. I can confirm that the feature works as expected and introduces no regressions in the plugins I've tested.
packages/plugin-ext-headless/src/hosted/node/plugin-host-headless-rpc.ts
Outdated
Show resolved
Hide resolved
packages/plugin-ext-headless/src/hosted/node/plugin-host-headless.ts
Outdated
Show resolved
Hide resolved
- define a 'theiaHeadlessPlugin' engine to identify headless plugins that support only the headless plugin host. Ordinary backend plugins that also run in the headless host can add a 'headless' entry-point in the 'theiaPlugin' object in package.json - update the Theia application webpack generator to handle packing the headless-plugin API init script - implement a headless entry-point in the PluginModel as a peer to frontend and backend entry-points - factor out common plugin manager behaviour into an abstract class - define distinct plugin manager (vscode/theia) and headless plugin manager implementations - similiarly for the HostedPluginSupport - define the minimal TerminalExt/Main APIs for access to the default shell to support the 'env.shell' API - implement nested Inversify container for headless plugins to isolate them and their plugin host from the connection-scoped plugins - add examples for headless plugin - add a "Greeting of the Day" example custom API provider - add a plugin-gotd example headless plugin that uses the custom API example to greet the world on activation and also illustrates simple vscode API usage Support headless entrypoint in VSCode plugins - support the headless entrypoint in an otherwise VS Code plugin - prefer the VS Code names for start/stop functions - update the example plugin to use be a dual VSCode/headless plugin using the vscode API in the usual backend entrypoint Support distinct and application-specific headless activation events - extend the PluginPackage interface to add a "headless" property initially defining only an optional "activationEvents" string array property for headless mode. The idea being that eventually other things like "contributions" might also be defined, here - support application injection of activation events that it will trigger in its plugins via the HeadlessHostedPluginSupport class's activateByEvent(activationEvent: string) API - adapt the example GotD plugin's package manifest to use the new "headless" property Differentiate provision of ext APIs to headless and backend plugin hosts - define a distinct headless ext API initialization function protocol and headless API initialization script path to target the headless plugin host specifically in the ext API provider - refactor the initialization of ext APIs in the plugin host to make use of this new distinction - update the Greeting-of-the-Day example API provider to support both headless and backend plugins - define the index for the common plugin-ext-headless API exports - fix up other minor details Inversify injection in the Plugin Host - implement an Inversify container in the plugin hosts - inject Ext interfaces and the RPC framework - get container modules in plugin entrypoints to configure the container - the plugin container module can provide for API initialization in a simpler, more reusable way than the current initialization function, which is still supported for backwards compatibility Signed-off-by: Christian W. Damus <cdamus.ext@eclipsesource.com>
Co-authored-by: Martin Fleck <mfleck@eclipsesource.com>
Fix underscoring of log message banner. Co-authored-by: Mark Sujew <mark.sujew@typefox.io>
e83cb9c
to
05acbec
Compare
Commit cffddc2 and successors are rebased to resolve the merge conflict by just deleting the changelog entry as it will be generated automatically. |
Celebration and congratulations are in order! |
What it does
Implement a new kind of plugin that I call "headless plugin" that is hosted in a child Node process like the current backend plugins except that this host is not started on a per-frontend-connection basis but exists independently of all such connections.
Thus it is "headless" as it will never know a Theia frontend.
This implements the functionality requested in #12946 and, because it makes no sense not to, #12959.
There are a few points to note about this implementation:
plugin-ext
package are refactored to pull up common functionality to reuse for headless plugins with the aim of promoting maintainability of both plugin stacks over the long term. I have endeavoured to maintain API compatibility for Theia extenders, but please do keep a sharp eye out for compatibility issuesa@theia/headless-plugin
API declaration is provided that is essentially a greatly stripped-down variant of@theia/plugin
, providing only capabilities that are available outside of frontend connections, and probably even less than that (see below for some possible follow-up in that area)headless-plugin
andheadless-plugin-ext
packages and work exactly as it does today. If the Theia team would rather just fold support for headless plugins into the existingplugin-ext
package (e.g. because that would eliminate the need for some of the refactorings done in this PR, actually simplifying the code and possibly making it more correct) then I would be happy to do thatheadless-plugin-ext
extension tries to match the organization of corresponding code in theplugin-ext
package, to make it easier to understand the similarities and differences. However, as there is no frontend to headless plugins, of course there are nobrowser
directories so some things (such asXyzMain
API) are organized differently, usually in thenode/
foldersMinimalTerminalServiceExt
type used by theAbstractPluginManagerExt
class to declare only what it actually needs to do its workHow to test
This PR includes new examples demonstrating the use case that motivated issue #12946 that can be used for testing. These are:
examples/api-provider-sample/
: a new example Theia extension that demonstrates how to provide a custom API object for plugins to use. This is important because the original use case motivating the headless plugins feature is one in which those plugins exist to plug into such application-specific APIs. The API provides a "Greeting of the Day" service with custom functions to call, classes to create, and call-backs for a plugin to register for eventssample-plugins/sample-namespace/plugin-gotd
: an example dual headless/backend plugin that uses the greeting-of-the-day API of the other new exampleand also illustrates simple usage of the API provided by Theia, itself, to headless plugins. It further demonstrates how Theia plugins may provide both backend and headless entry-points to run in both plugin hostsTo test the intended use case of this feature, simply copy the
plugin-gotd-v1.44.0.tgz
tarball from the sample plugin into yourplugins/
folder and start up the Browser Backend. You should see the new headless plugin host start up, load this plugin, and output from it on the console using[GOTD]
tags to help you locate them in the output. Then hit the frontend in your browser to see that the connection-scoped backend plugin host does its usual thing, and that the GOTD plugin's backend entry-point runs in this context, printing messages tagged with[GOTD-BE]
to distinguish them from messages printed by the headless plugin.Then, of course, test the same behaviour in Electron as much as makes sense.
Apart from that, please go as crazy as you like in regression testing the usual plugin capabilities of Theia.
Follow-ups
someno attempt has been made to supporta fewany core Theia services in the headless plugin host that should be available generally, outside of frontend connections.I don't really know how to test most of that so there may be problems found by early adopters that we need to fix. (Consider what default API should be supported for Headless Plugins #13292)also there may be other core Theia services that should be exposed in the new(Consider what default API should be supported for Headless Plugins #13292)@theia/headless-plugin
API that currently are not, contribution points and activation events to support in the package manifest that currently are not implemented. I should like to leave it to adopters to identify real-world needs in these areas that we can add latertheia
orvscode
API namespace) to plugins but relies on extensions and other plugins to provide APIs, as demonstrated by the new example GotD plugin and the API that it uses provided by the new example extension. It is an area of future consideration whether to provide some default API namespace and what capabilities it should offer, as use cases are discovered (Consider what default API should be supported for Headless Plugins #13292)Review checklist
Reminder for reviewers
Updated 5 Dec 2023
This description is amended to rebrand the new kind of plugin as "headless plugin" (was "base plugin") and to adapt the test instructions for the now dual nature of the example GotD plugin, which previously supported only headless mode but now also backend (of a frontend connection) mode.
Updated 11 Dec 2023
This description is amended to reflect the withdrawal of the default API namespace that was provided to headless plugins.
Updated 21 Jan 2024
This description is amended to link to the follow-up tickets raised for follow-ups in the relevant section of the template.