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

PoC - cli: filter Theia extensions #9309

Closed
wants to merge 1 commit into from

Conversation

paul-marechal
Copy link
Member

@paul-marechal paul-marechal commented Apr 6, 2021

Our generators currently collect any Theia extension installed in
node_modules and mount them into your application without leaving you
much of a choice.

An alternative could be to create your own generators, but this is a lot
of maintenance work.

This commit simplifies the process of controlling what ends up in your
applications by adding new loading strategies:

  • all: Like before, use everything found in node_modules
  • explicitDependenciesOnly: only use what is defined in your
    dependencies

This is configurable from an application package.json through the
theia.extensions.loading.strategy field.

In addition to those strategies, you can also specify an includes list
and an excludes list. Using those, you can define regular expressions to
filter extensions even more precisely. The matching will be done on the
candidate inversify modules path.

When using the explicitDependenciesOnly the includes list will be
evaluated as "in addition to explicit dependencies". If you want the
includes list to be absolute again, use the default 'all' strategy so
that only what's defined in includes is included.

Note that preventing an extension from having its inversify modules
loaded won't prevent it from being included in your bundles. Bundling
should also mostly work no matter what you exclude, but if another
extension was relying on a given Symbol it will most likely break at
runtime. In such a case it is your responsability to bind the missing
symbols using a custom Theia extension, specific to your use-cases.

How to test

Edit examples/browser/package.json to have the following fields:

  "theia": {
    "extensions": {
      "loading": {
        "strategy": "all",
        "includes": [],
        "excludes": [
          "^@theia/typehierarchy"
        ]
      }
    },
    // ...

After rebuilding the browser app, inspect the src-gen/frontend/index.js and src-gen/backend/server.js files and make sure that no modules are matching the excludes list.

To test the explicitDependenciesOnly you can remove @theia/terminal from the browser app dependencies and make sure it doesn't get included in the previously mentioned files, but it should be included when using the all strategy.

Review checklist

Reminder for reviewers

@paul-marechal
Copy link
Member Author

#9300 prompted me to find a solution for when we need typings for a package, but we might not want to bring all of its contributions into our app because it is a transitive dependency.

This PR gives more control into what gets loaded through inversify into your application, at the extension/module level.

I want to get feedback if it is something others here also wanted to be able to do.

@paul-marechal
Copy link
Member Author

This mechanism should be complementary to #9069. I discussed on this issue having a build-time mechanic, and while this PR is different from what I described back then, it should still be beneficial to application developers.

I also remember @thegecko had troubles with @theia/plugin-ext pulling @theia/terminal which polluted their application transitively. For that issue related to plugins #8180 might be a better solution long term. But being able to explicitly exclude things from applications should still be a useful power.

@thegecko
Copy link
Member

thegecko commented Apr 7, 2021

Cc @arekzaluski

@paul-marechal
Copy link
Member Author

I simplified the logic and the options a bit. See commit message/PR description.

Our generators currently collect any Theia extension installed in
node_modules and mount them into your application without leaving you
much of a choice.

An alternative could be to create your own generators, but this is a lot
of maintenance work.

This commit simplifies the process of controlling what ends up in your
applications by adding new loading strategies:

- all: Like before, use everything found in node_modules
- explicitDependenciesOnly: only use what is defined in your
  dependencies

This is configurable from an application package.json through the
theia.extensions.loading.strategy field.

In addition to those strategies, you can also specify an includes list
and an excludes list. Using those, you can define regular expressions to
filter extensions even more precisely. The matching will be done on the
candidate inversify modules path.

When using the `explicitDependenciesOnly` the includes list will be
evaluated as "in addition to explicit dependencies". If you want the
includes list to be absolute again, use the default 'all' strategy so
that only what's defined in includes is included.

Note that preventing an extension from having its inversify modules
loaded won't prevent it from being included in your bundles. Bundling
should also mostly work no matter what you exclude, but if another
extension was relying on a given Symbol it will most likely break at
runtime. In such a case it is your responsability to bind the missing
symbols using a custom Theia extension, specific to your use-cases.

Signed-off-by: Paul Maréchal <paul.marechal@ericsson.com>
@sdirix
Copy link
Member

sdirix commented Apr 8, 2021

This mechanism should be complementary to #9069. I discussed on this issue having a build-time mechanic, and while this PR is different from what I described back then, it should still be beneficial to application developers.

I agree! This PR should work well in combination with #9317 to allow application developers to exclude whole extensions at build time as well as individual contributions from included extensions at runtime.

@paul-marechal
Copy link
Member Author

I'll hold this for now.

@paul-marechal
Copy link
Member Author

See #9317

@paul-marechal paul-marechal deleted the mp/extension-filtering branch August 19, 2021 15:03
@paul-marechal paul-marechal restored the mp/extension-filtering branch August 19, 2021 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants