Skip to content
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

Theia extensions and extensibility #1347

Closed
benoitf opened this issue Feb 22, 2018 · 10 comments
Closed

Theia extensions and extensibility #1347

benoitf opened this issue Feb 22, 2018 · 10 comments
Labels
design issues related to design extension system issues related to the extension system question user / developer questions

Comments

@benoitf
Copy link
Contributor

benoitf commented Feb 22, 2018

Hello, here is an issue to discuss the topic of the Theia extensions, the current status and from my point of view, some current areas where it could be improved.

Theia extensions and extensibility.

Currently, in Theia, all extensions and core are transformed at the end in one big single bundle.js file generated for the frontend side or the backend side.

example on Frontend source code generator and Webpack source code generator

It means that adding a frontend extension is resulting in having webpack called again to have bundle.js file updated, then browser can be refreshed and the extension is there. It also means that adding an extension is implying that all dependencies of that extension (which is currently an npm package) will be downloaded as well.

It leads to some areas where I think we could have improvements:

A. Self-contained extension

There is no "self-contained" extension right now. It always requires to download the dependencies of the given extension and a build step (webpack). (stick to npm module)
issues:

  • Npm registry may have networking problems at the time of the extension is being installed/ offline mode issue/ public internet access issue, proxy issue, etc.
  • it's not possible to upload a single "zip/self-container extension archive" and load it dynamically.

It means:

  • To be able to load dynamically an extension without requiring webpack step to rebuild the "full IDE".
  • Not rely on network after "extension" is packed/added on the filesystem

B. Isolation

JavaScript Sandboxing/isolation between extensions is not possible

  • all extensions are loaded in the same scope and in the same thread. This allows to extend everything but it also enable to break everything.
  • can two separate extensions collide if they're installed together ? while they're working fine if only one is installed.

C. Memory consumption

  • all extensions are loaded into the memory even if the extension will not be called. (there is a single bundle.js that is loaded)
    • no lazy loading/activation events for an extension like loading on demand the .js file of the extension

D. Scalability

  • Each new extension installed will increase the build time. Many extensions => build time bigger.

E. Polyglot

Is it possible to easily write extensions in other languages than TypeScript.
Kotlin, Ceylon, etc.

F. Reporting on extension time

for example on Atom there is https://atom.io/packages/timecop allowing to track
- Startup time
- Compile cache
- Package loading time
- Package activation time
- Theme loading time
- Theme activation time

Appendix : Area of experiments?

  • each extension could produce its own 'binary' / webpack result ?
  • do not embed 'core dependencies' into each extension webpack bundle.js
  • use of peerDependencies ? allowing to track for which version of dependencies it's supported
  • external mode in webpack
  • lazy loading of the extension (could be event based)
@benoitf benoitf added question user / developer questions design issues related to design labels Feb 22, 2018
@akosyakov akosyakov added the extension system issues related to the extension system label Feb 22, 2018
@svenefftinge
Copy link
Contributor

svenefftinge commented Feb 23, 2018

Thanks, Florent.
I am adding some comments to your observations below.
For the extension system, we decided to reuse the existing knowledge and infrastructure of npm, semver, npm registries and others. Because it not only makes it easier to understand it but also lets us reuse mature technology that solves the same problems.

Currently, in Theia, all extensions and core are transformed at the end in one big single bundle.js file generated for the frontend side or the backend side.

This only happens for the frontend. The backend does module resolution at runtime.
Browsers cannot do node module resolution, because they can not traverse directories on servers, which is why the resolution needs to happen once. This is what webpack does.

It means that adding a frontend extension is resulting in having webpack called again to have bundle.js file updated, then browser can be refreshed and the extension is there.

True. Dynamically updating a single module in a running application is very complex and prone to errors, because of state management.

It also means that adding an extension is implying that all dependencies of that extension (which is currently an npm package) will be downloaded as well.

Of course, I don't see how this could ever be avoided.
But they are only downloaded if they are not already there.

A. Self-contained extension
There is no "self-contained" extension right now. It always requires to download the dependencies of the given extension and a build step (webpack). (stick to npm module)

I don't understand what the difference between downloading an 'extension' or downloading an 'npm package' is. In both cases a single archive is downloaded to be executed.

Npm registry may have networking problems at the time of the extension is being installed/ offline mode issue/ public internet access issue, proxy issue, etc.

We don't rely on npm.js. That is just a default. You can specify a different npm registry.
If you were building your own extension registry, it could be offline, too.

it's not possible to upload a single "zip/self-container extension archive" and load it dynamically.

When you publish to an npm-registry you are uploading a single 'zip'. Self-contained is not desirable because an extension might have dependencies to other extensions.

To be able to load dynamically an extension without requiring webpack step to rebuild the "full IDE".

Seems like the 'dynamic' part is important. Could you outline why that is? Isn't it just important to make it a smooth and quick transition? IMHO whether modules are resolved once on the backend or at runtime in the browser is an implementation detail.

Not rely on network after "extension" is packed/added on the filesystem

There are two steps at the moment.

  1. npm install will download the dependencies from a registry (could be a local one).
  2. the Theia application manager updates the bundle.js

B. Isolation
JavaScript Sandboxing/isolation between extensions is not possible

Unlike the extension model of VS Code where every extension runs in their own sandbox, we decided to go with a model like we have in Atom or in the traditional Eclipse IDE. This means that extensions are first class citizens and can integrate deeply. Of course, there are trade-offs, like that extensions could possibly do more harm because they have much more power.
That said, if you want run code in isolation you can always spawn a webworker.

all extensions are loaded in the same scope and in the same thread. This allows to extend everything but it also enable to break everything.
can two separate extensions collide if they're installed together ? while they're working fine if only one is installed.

Possibly, yes. As I said it is like in Eclipse and Atom.

all extensions are loaded into the memory even if the extension will not be called. (there is a single bundle.js that is loaded)
no lazy loading/activation events for an extension like loading on demand the .js file of the extension

In Eclipse there was a lot of complexity added to make the lazy loading stuff work. We are all forced to describe UI contributions in clumsy XML. In the end, we figured that most of the extensions were always loaded, anyway.
That said I would like to improve this especially to speed up startup time.

D. Scalability
Each new extension installed will increase the build time. Many extensions => build time bigger.

In theory, yes, but in practice, webpack is quite fast. Again it is a trade-off. I believe that installing an extension is a task where a couple of seconds (< 20) install time is ok, because you don't do that all day long. But we have to keep an eye on this, for sure.

E. Polyglot
Is it possible to easily write extensions in other languages than TypeScript.
Kotlin, Ceylon, etc.

Yes, as long as it is JavaScript in the end.

F. Reporting on extension time
for example on Atom there is https://atom.io/packages/timecop allowing to track

  • Startup time
  • Compile cache
  • Package loading time
  • Package activation time
  • Theme loading time
  • Theme activation time

Maybe you want to put this into its own ticket, so it doesn't get lost while discussing the basic decisions of the extension system.

@benoitf
Copy link
Contributor Author

benoitf commented Feb 26, 2018

In order to better exchange on this issue, I decided to do a POC to make it easier to understand but also to show that it's not reinventing the wheel as it's using the same tools.

Webpack is also promoting: (see bottom for the links)

  • Authoring libraries patterns [1]
  • Code splitting [2]

So plugins/extensions could be "separate code" reusing "Theia" libraries
(it's done by using externals webpack approach)

So for example, for an hello world extension, webpack configuration is making inversify and theia code loaded as an external dependency, thus the final webpack output is mainly including the "extension's code" (there are few extra code added to wrap the code)

Summary of the POC:

We have a Theia running, and I add in index.html an external script helloworld.js file served by rawgit which is a webpack generated file. (for the POC it was avoiding to use a loader to make it load lately but it would act the same way)
This hello world extension is providing new inversify injectable/injected stuff : a command contribution

When I click on a custom "install plugin" , it creates an Inversify container and it loads the extension containermodule. We can get the new provided contribution. Of course, for real we should get the "frontendApplication" container and use that container so extension has access to the correct container.

Here is a simple screencast
js-load-time

In short:

Each extension is a webpack bundle.
Theia's code should not be included in this bundle (externals)
We could still use a single bundle.js with a single file but it is also letting a way to load these extensions dynamically and with no "extra build" step.

About the issue

I believe that installing an extension is a task where a couple of seconds (< 20) install time is ok, because you don't do that all day long. But we have to keep an eye on this, for sure.

For che, loading a workspace at demand implies that plugins are also "on demand". Then, waiting 20 seconds before being able to open a workspace only to add the required plugins is far too long.
Ideal time to have a Che workspace running is now < 5 seconds.

Appendix : Links

[1] https://webpack.js.org/guides/author-libraries/
[2] https://webpack.js.org/guides/code-splitting/

@akosyakov
Copy link
Member

akosyakov commented Feb 26, 2018

Each extension is a webpack bundle.

Do you mean that the backend part should be bundled as well? Webpack is used only for the frontend.

Theia's code should not be included in this bundle (externals)

We need to ensure that anything which is injectable should be loaded once and in the proper order, otherwise DI and instanceof checks are not going to work.

We could still use a single bundle.js with a single file...

It would be interesting to know is there a performance difference between bundling like now or bundling pre-bundled extensions. If there is we can consider having some special tag for publishing minified obfuscated bundled artifacts, i.e. @theia/core: ^0.3.0-bundle.

...but it is also letting a way to load these extensions dynamically and with no "extra build" step.

For the frontend part - yes. For the backend, if like now the server restart is required (and the page reload). We could think about bundling it as well.

@benoitf
Copy link
Contributor Author

benoitf commented Feb 26, 2018

here is the warnings with Webpack 4.0

WARNING in asset size limit: The following asset(s) exceed the recommended size limit (244 KiB).
This can impact web performance.
Assets: 
  912ec66d7572ff821749319396470bde.svg (434 KiB)
  bundle.js (3.75 MiB)
  vs/language/css/cssWorker.js (466 KiB)
  vs/editor/editor.main.js (1.72 MiB)

WARNING in entrypoint size limit: The following entrypoint(s) combined asset size exceeds the recommended limit (244 KiB). This can impact web performance.
Entrypoints:
  main (3.75 MiB)
      bundle.js


WARNING in webpack performance recommendations: 
You can limit the size of your bundles by using import() or require.ensure to lazy load some parts of your application.
For more info visit https://webpack.js.org/guides/code-splitting/

@benoitf
Copy link
Contributor Author

benoitf commented Feb 26, 2018

@akosyakov for now I've mainly talk on the frontend part. I will complete with backend items soon

@benoitf
Copy link
Contributor Author

benoitf commented Feb 27, 2018

So, for the backend side, extension can also be webpack generated files

I've used a dummy echo service with code

@injectable()
export class EchoBackendApplicationContribution implements BackendApplicationContribution {

    configure(app: express.Application): void {
        app.get('/echo', (request, response) => response.send("hello world"));
    }

}

and with a inversify module providing it as well

and in webpack configuration I've specified commonjs as libraryTarget and node as target

 target: "node",
  output: {
    path: path.resolve(__dirname, 'dist'),
    filename: 'echo-backend-extension.js',
    library: 'echoBackendExtension',
    libraryTarget:'commonjs'
  },
  externals: [
    "inversify",
    /^\@theia\/.+$/
  ],

This file is loaded in addition to server.jsand main.js and the Hello service is enabled.

image

I've noticed that when we perform code like

export default new ContainerModule(bind => {
    bind(Foo).to(Bar);
});

the Foo Symbol needs to be correctly imported else it ends into a binding that is not visible by others

@svenefftinge
Copy link
Contributor

So, for the backend side, extension can also be webpack generated files

Why would we want to do that?

@benoitf
Copy link
Contributor Author

benoitf commented Mar 2, 2018

to separate assembly/build time and load time only

you build plugins/extensions and you publish them. IDE is only loading these extensions/plugins directly, not rebuilding/downloading dependencies/run post-hook, etc

@svenefftinge
Copy link
Contributor

svenefftinge commented Mar 3, 2018

That is already the case for the backend. There is no build involved for the backend extensions.

@benoitf
Copy link
Contributor Author

benoitf commented Mar 12, 2018

closing it by opening a more specific issue #1482 as discussed in the last weekly call

@benoitf benoitf closed this as completed Mar 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design issues related to design extension system issues related to the extension system question user / developer questions
Projects
None yet
Development

No branches or pull requests

3 participants