eclipse-theia / theia

Eclipse Theia is a cloud & desktop IDE framework implemented in TypeScript.
http://theia-ide.org
Eclipse Public License 2.0
19.86k stars 2.48k forks source link

All contributions in the dependency tree are loaded #2981

Open thegecko opened 5 years ago

thegecko commented 5 years ago

As a developer using Theia, I would expect to only see contributions loaded which are in the packages I explicitly add to the application as dependencies. This isn't the case as all other contributions found in the dependency tree are also included.

For example, the theia/callhierarchy package can be omitted from the dependencies section of an example package.json and I would expect it to not be loaded. However it still appears because theia/core depends on it.

marcdumais-work commented 5 years ago

However it still appears because theia/core depends on it.

That particular case sounds like a bug. By definition, the core extension should probably not depend on anything else, specially something somewhat peripheral, like the callhierarchy view, that would not be needed for many Theia applications.

Generally, it seems helpful to automatically resolve/pull-in Theia extension dependencies, even when those dependencies are not stated explicitly, for a given Theia application.

OTOH, such missing dependencies can cause issues; it seems the build takes the dependencies into account, and makes sure they are built first. In this context, not having them explicitly stated can cause build issues. e.g.: #2888

thegecko commented 5 years ago

I agree my example is probably an architectural bug, but I can still see scenarios where a dependency between extensions may need to exist for e.g. type imports without wanting the contributions to be loaded.

However I may have uncovered the only area where this issue exists :)

thegecko commented 5 years ago

Apologies, the dependency seems to be between theia/typescript and theia/callhierarchy which makes sense.

thegecko commented 5 years ago

Another situation where this issue exists is if a developer wants the monaco extension, but not the outline-view extension. monaco depends on outline-view in order to import the interface to act upon events, however this means both extensions are always loaded.

akosyakov commented 5 years ago

As a developer using Theia, I would expect to only see contributions loaded which are in the packages I explicitly add to the application as dependencies.

It is intended: If extension A depends on extension B then extension B should be loaded before A, otherwise, A cannot rely on runtime services of B. If extension package B is optional then it should not be listed as a dependency in package A.

For example, the theia/callhierarchy package can be omitted from the dependencies section of an example package.json and I would expect it to not be loaded. However it still appears because theia/core depends on it.

theia/core does not depend on theia/callhierarchy, please look at package.json of theia/core, something else is pulling it.

marcdumais-work commented 5 years ago

Another situation where this issue exists is if a developer wants the monaco extension, but not the outline-view extension. monaco depends on outline-view in order to import the interface to act upon events, however this means both extensions are always loaded.

This too, sounds like it's a matter of tweaking the dependencies, so that they make sense, rather than a problem with the way we automatically resolve Theia extensions dependencies. i.e. I think we would need to modify the monaco extension so it does not depend on callhierarchy, if we want to be able to have the former without the later, in a Theia app.

thegecko commented 5 years ago

theia/core does not depend on theia/callhierarchy, please look at package.json of theia/core, something else is pulling it.

I noticed that and commented previously. The dependency is with theia/typescript, not theia/core - my mistake.

OK, if this is the intended behaviour, areas of coupling I've noticed which need addressing are:

Every one of these soft dependencies bring UI components into Theia which may not be desirable depending on your implementation.

akosyakov commented 5 years ago

Sounds good, usually it can be done by extracting a new extension, like typescript-callhierarchy.