Skip to content

Code Improvement Areas

Paul Maréchal edited this page Dec 20, 2022 · 2 revisions

As in any piece of software, there are areas in the Theia source code where we didn't quite get the design right the first time. This document lists area where we think changing the design would make it simpler to evolve the code and deliver better quality. The idea is to maintain a top-3 to be fleshed out into epics for adopters/contributors to sign up for like we did for VS Code compatibility.

Tasks

The tasks system suffers form the fact that we do not distinguish between the text in a tasks.json file and the task configuration objects that we use internally. We start our task object hierarchy with TaskCustomization which is basically just the tasks.json schema expressed as a Typescript interface. I believe the code would become much easier to understand (and thus less buggy) if we clearly separated the two concepts. Task Customizations from json files are applied to the task objects defined by the system or plugins, but customizations and configuratitions are not the same. As a quick example: the values for taskGroup in tasks.json is build | test whereas in the API there are move values allowed. This leads to all kinds of gymnastics when converting types.

Debugger

Currently we have a DebugService and a PluginDebugService which entirely replaces it. The point of variability here should not be the debug service, but the debugger contributions. Here's an issue related to this: https://github.com/eclipse-theia/theia/issues/11121

Inversify

We don't use the most recent version of Inversify

We currently use Inversify ^5 which doesn't really support asynchronous injections/constructions.

We misuse post-constructors

Despite this, we have a bunch of asynchronous post constructors which is problematic as Inversify doesn't await them: https://github.com/eclipse-theia/theia/issues/10788

Upgrade to Inversify 6 is not trivial

Since then Inversify 6 got released and supports asynchronous concepts, but our codebase doesn't really work with it. e.g. We sometime inject references to promises but newer Inversify thinks the promise needs to be resolved before constructing the component, leading to errors.

Moreover we'd need to make sure that there are no regressions using the new asynchronous facilities provided by Inversify, as the app may initialize in a different way.