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

cli: add check for detecting multiple versions of the same theia extension #12572

Closed
gergaudf opened this issue May 26, 2023 · 11 comments · Fixed by #12596
Closed

cli: add check for detecting multiple versions of the same theia extension #12572

gergaudf opened this issue May 26, 2023 · 11 comments · Fixed by #12596
Labels
proposal feature proposals (potential future features) theia-cli issues related to the theia-cli

Comments

@gergaudf
Copy link

gergaudf commented May 26, 2023

It seems that a Theia extension could not exist in 2 different versions at runtime as one would override the other in the Dependency Injection context, leading to unexpected behavior during runtime (or is it overridden during the Theia CLI build?). In our case, we develop several Theia extensions and we systematically have to check that they are all on a single version from the product's dependency graph. On one hand, we have a tool similar to the dependency check utility which require us to know in advance all Theia extensions in our dependency graph, on the other hand, the Theia CLI filters the dependency graph to get all Theia extensions during the build.

Feature Description:

Would it be possible for the Theia CLI to automatically detect if a Theia extension with 2 different versions is present in the dependency graph during the build? using an option like "-ensure-theia-extensions-version-uniqueness" would fail the build if this is the case.

@vince-fugnitto
Copy link
Member

vince-fugnitto commented May 26, 2023

@gergaudf you can use yarn theia --help to discover other cli options, one of them being:

theia check:theia-version - Check that all dependencies have been resolved to the same Theia version

@gergaudf
Copy link
Author

@gergaudf you can use yarn theia --help to discover other cli options, one of them being:

theia check:theia-version - Check that all dependencies have been resolved to the same Theia version

Thanks @vince-fugnitto ,
I guess the requirement here is how do we reproduce this mechanism for any Theia extension, including the ones we are developing, not only the @Theia ones? On our project, the Theia extensions we develop from scratch, not scoped with "@Theia", can be imported as a first level dependency when building a Theia application, or can be brought transitively by another dependency. We started to loose track of them and developed a tool that parses the yarn.lock file and check that our dependencies does not exist with multiple versions. This scripts requires to know in advance all our Theia extensions, to be looked for. So we decided that we would parse the content of the Theia's application's package.json and transitively establish the dependency graph to then filter out all the Theia extensions (these packages contains a "theiaExtensions" attribute).
This seems to be pretty similar to what the Theia CLI is doing when building an application.
On the other hand, it seems that having 2 different versions of a same Theia extension triggers unpredictable behavior at rumime (TBC) for most of the Theia extensions. So I was wondering if this check could be included directly in the Theia CLI instead of developing our own tool.

@vince-fugnitto
Copy link
Member

@gergaudf I'm not really sure I follow, the CLI should help with discovering and identifying multiple versions of extensions, for both the ones provided by the framework by default and new extensions developed by extension developers given that the extension itself pulls @theia dependencies. I'm not really sure how you end up in a state where you're pulling multiple versions of your own extension and itself not declaring any dependency to any existing @theia dependency.

On the other hand, it seems that having 2 different versions of a same Theia extension triggers unpredictable behavior at rumime (TBC) for most of the Theia extensions. So I was wondering if this check could be included directly in the Theia CLI instead of developing our own tool.

As mentioned there are already mechanisms to identify when multiple versions are pulled, we just need to use them.
In this case I built a "hello world" example extension and pulled both latest and 1.35.0. The tool correctly identifies that two different versions are not supported:

✅ Found 3 workspaces
✅ Found 25 dependencies
🟠 Found 1 issues

#1  @theia/application-manager [theia-version-mix]
    error Mix of @theia/* versions found.
    1.37.2 in node_modules/@theia/application-manager/package.json
    1.35.0 in node_modules/@theia/getting-started/package.json
    1.35.0 in node_modules/@theia/keymaps/package.json
    1.35.0 in node_modules/@theia/userstorage/package.json


⛔ A mix of Theia versions is very likely leading to a broken application.
ℹ️  Use yarn why <package-name> to find out why those multiple versions of a package are pulled.
ℹ️  Try to resolve those issues by finding package versions along the dependency chain that depend on compatible versions.
ℹ️  Use resolutions in your root package.json to force specific versions as a last resort.

error Command failed with exit code 1.

@vince-fugnitto vince-fugnitto added theia-cli issues related to the theia-cli 🤔 needs more info issues that require more info from the author labels May 30, 2023
@gergaudf
Copy link
Author

Thanks for your answer @vince-fugnitto.
Could you please confirm the 2 following points:

  1. if I am in the following scenario:
  • myTheiaApp depends on myExtensionA@0.0.2 which depends on @theia/core@1.37
  • myTheiaApp depends on myExtensionB@0.0.1 which depends on myExtensionA@0.0.1, which depends on @theia/core@1.37
    the existing CLI tool is supposed to throw an error as myExtensionA@0.0.1 and myExtensionA@0.0.2 will conflict?
  1. the CLI tool able to detect all Theia extensions (not only the @Theia ones) in the dependency graph without having to explicit list them?
    I tried the CLI tool but couldn't make it detect my conflict. I will give it another try.

@vince-fugnitto
Copy link
Member

@gergaudf no problem, the current CLI tool is mainly a helper for identifying the same @theia dependency at multiple versions as this would cause issues. Since in your setup you have multiple versions of your own extension but it itself uses the same @theia dependency then its likely the tool confirms it's fine as that's what it's job is to do. I can't really confirm as I don't have a similar setup myself.

I believe it may be more of an issue with your development setup, for example you may want to use a monorepo to develop extensions that rely on eachother together and so on, or use latest tags instead of hardcoding to a version and then finding duplicates. I'm not sure what the framework should or could provide as a utility to identify multiple versions of custom extensions.

If it's something you want to investigate please don't hesitate in providing a pull-request. In addition, I believe there exists utilities already to discover duplicates, for example npm itself has the ability to identify dupes with npm find-dupes.

@sdirix
Copy link
Member

sdirix commented May 30, 2023

Hi @vince-fugnitto, I think that this would make a nice addition to the Theia CLI.

It's true that it's already possible to find duplicates with existing tools, however in many cases duplicates are not a problem. For example consuming two versions of lodash, while not ideal, does not lead to problems at runtime. Therefore existing tooling can't be used as is but must be further processed.

While the example given in the thread focuses on an accidental duplication of own packages, this can also be taken further to 3rd party dependencies unrelated to mono-repo management. For example EMF Cloud offers a number of Theia extensions on which own packages typically require directly but also transitively. It would be nice to have a convenient check for this use case too.

I think the Theia CLI or the Theia source gen would be great places for this check. They already handle dependencies so conceptually they "simply" need to be extended. Any occurrence of a Theia extension duplication will most certainly lead to runtime problems which is a Theia specific property. All Theia based apps would profit from such a check and many will implement it redundantly if it's not provided.

As we already offer a Theia specific dependency check with theia check:theia-version, which could as well just be implemented with existing npm package tools and further processing, I would argue in favor of also providing something like theia check:extension-duplicates.

@vince-fugnitto vince-fugnitto added proposal feature proposals (potential future features) and removed 🤔 needs more info issues that require more info from the author labels May 30, 2023
@vince-fugnitto
Copy link
Member

@sdirix sounds good, if you see a need please feel free to enhance #11483 to offer the ability to check for extension duplicates. I assume you'll only check for packages where theiaExtensions is declared in the package.json (since we only want to check for theia extensions)?

@vince-fugnitto vince-fugnitto changed the title Theia CLI build failure when a same Theia extension is present in multiple versions in the product cli: add check for detecting multiple versions of the same theia extension May 30, 2023
@sdirix
Copy link
Member

sdirix commented May 31, 2023

Yes, that's the idea 👍

@gergaudf
Copy link
Author

gergaudf commented May 31, 2023

Couldn't this check be done systematically by the Theia CLI build command instead of check:dependencies? I may misunderstand the rational here, so please correct me if I am wrong: if any theia extension cannot coexist in different versions at runtime, shouldn't we avoid building Theia application in this case?

@sdirix
Copy link
Member

sdirix commented May 31, 2023

I can see arguments for both, a specific check or adding this check to the build. Note that the source of truth for dependencies also differs between frontend and backend. Frontend dependencies are determined at build (i.e. bundle) time while the backend entries are determined during build time but the resolving and execution happens at runtime. Technically packed Electron builds also transform the dependencies and would require another check.

As we already have the Theia version check as part of the Theia CLI it would be consistent to also do the duplicate Theia extensions check there. Otherwise the Theia version check could also have been part of the build. So personally I would just add it to the CLI too. However adding it to the build would also be fine for me. Depending on the effort we could even do both.

Is there a specific preference on your side @vince-fugnitto?

@vince-fugnitto
Copy link
Member

Is there a specific preference on your side @vince-fugnitto?

@sdirix I'd be fine to keep it as a CLI tool to start with, downstream can easily call it during their build process if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal feature proposals (potential future features) theia-cli issues related to the theia-cli
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants