Theia: Discuss and declare the way how we handle transitive dependencies in Theia extension

[kittaakos]

Currently, for instance, the monaco Theia extension does not declare transitive dependency to the inversify library but uses it. Right now, it does not do any harm and works as expected due to the dependency hoisting (in Lerna) but in the future removing dependencies from the core part might break client extensions.

Discuss how to deal with these. Explicitly declaring dependencies in the extension might solve all the issue. This ticket is just a placeholder and a reminder task, any recommendations are welcomed.

[svenefftinge]

I vote for explicitly adding dependencies that are directly used.

[hexa00]

I'm not sure how to enforce that however.. ideas?

[akosyakov]

I don't see an issue here. @theia/monaco depends on inversify transitively via @theia/core. It should work even without Lerna.

[hexa00]

I think he means it what if some extension has a transitive dependency to something via @theia/core and we remove for some reason that dependency in core.

Of course it's not a problem with inversify but.. something else might be an issue?

[akosyakov]

I think it should be resolved when it happens for some dependency, not generally.

Otherwise, consider a dependency to @phosphor/widgets from @theia/core that transitively imports 10 other phosphor packages. With the explicit declaration, we should duplicate all of them in all extensions and make sure that all declared versions in sync. Right now it is just one dependency in @theia/core.

[hexa00]

The idea would be to have only direct dependencies explicit (anything your code does import on directly) ... so if your code uses phosphor you only include phosphor not it's dependencies...

I do agree that the versioning might be an issue.

[akosyakov]

It is not possible. @phosphor/widgets uses code from dependencies in API, so the user code should declare its dependencies.

[svenefftinge]

It would be nice to have the notion of a reexported dependency. But I see that in practice the lack of central version management would be a real bummer if we were forcing us to redeclare the same five dependencies in every extension. @kittaakos what was the issue again, that caused us to discuss this?

[kittaakos]

The issue was: how can I depend on inversify in my extension even if I do not have a direct dependency on that? I know, it is because of lerna and the shared dependencies.
And the second is, that is the more problematic one; I am using a transitive dependency from theia/core and what if that dependency get removed, or just its API changes.

tl;dr:
Long story short; how to ensure not to break client extensions.

[kittaakos]

This is an issue in case of the packaged electron applications.

Let assume the following:

• One is writing an electron application that depends on package A. This dependency is explicitly declared in the package.json of the electron application.
• Package A uses package B, and this, dependency B, is declared explicitly in the package.json of package A.

When one installs A in the electron application, then the node_modules folder structure will be something like this:

node_modules
|
+-A
| |
| +-node_modules
|  |
|  +-B
|
+-B

So one can call require('B') from the electron application during the development even if there is no stated, clear dependency to package B from the application code.

The electron builder will get rid of these duplicate dependencies. After running the electron builder, the node_modules folder structure will be something like this:

node_modules
|
+-A
  |
  +-node_modules
   |
   +-B

So all the transitive dependencies which are used in the client code have to be declared explicitly.

I think this behavior determines whether transitive dependencies have to be stated in the client's package.json or not; yes they have to be.

CC: @svenefftinge, @hexa00, @akosyakov

[hexa00]

Related to: https://github.com/yarnpkg/yarn/issues/4070 ?

Sounds like we should workaround that rather than declare everything ?

Could we just flatten the node_modules structure after the electron_builder ?

[svenefftinge]

I think @kittaakos doesn't use yarn workspaces or hoisting in his example app.
I noticed that yarn and npm install behave differently with regards to nested node_modules folders. Did you check both?

[kittaakos]

doesn't use yarn workspaces or hoisting in his example app.

That is correct. I do not even check-out the source code.

I noticed that yarn and npm install behave differently with regards to nested node_modules folders.

Great, so others have noticed this too. I have to look into it.

Did you check both?

No, not yet. I have to. Thanks for the pointer!