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

Built-in extensions using federated dependencies #9310

Merged
merged 10 commits into from Nov 12, 2020

Conversation

jasongrout
Copy link
Contributor

@jasongrout jasongrout commented Nov 10, 2020

References

This fixes the last half of #9289. It allows built-in extensions to use dependencies shared by federated extensions. It also cleans up the built-in extension configuration.

Code changes

  • Load federated extensions in a bootstrap phase, before the main application imports compiled plugins and starts the application. This ensures that if a compiled plugin requires something provided by a federated extension, it will be able to get it.
  • Treat compiled extensions (including core extensions) and federated extensions similarly when it comes to sharing config. We always default to sharing all direct dependencies, and we allow for any plugin to have a customized jupyterlab.sharedConfig key listing sharing config that overrides these defaults.

We are careful to merge the extension sharing config for the compiled build. We never override the core set of resolutions (those are always shared), and any shared config that says to bundle a dependency takes precedence over a config saying not to bundle a dependency.

User-facing changes

Compiled extensions can use federated dependencies. For example, with this, a user should be able to jupyter labextension install an ipywidgets plugin with the jupyterlab_widgets federated extension base widget package. The ipywidgets plugin should be able to use the ipywidgets manager provided by the federated extension.

Backwards-incompatible changes

There should not be any substantial changes in this. It is just reorganizing, deleting duplicate code (fixing a bug?), and updating code to be a bit more modern.
This ensures that the extensions can be packed into larger bundles, rather than individually bundled. The assumption here is that built-in things will all be needed, or at least that relatively few will be disabled.

Individual extensions can still load parts of themselves asynchronously to be separately bundled and loaded on-demand (for example, codemirror themes, the codemirror vim extension, etc.).
@jasongrout jasongrout added this to the 3.0 milestone Nov 10, 2020
@jupyterlab-dev-mode
Copy link

Thanks for making a pull request to JupyterLab!

To try out this branch on binder, follow this link: Binder

externalExtensions are extra extensions automatically added to extensions/mimeExtensions data
This makes built-in extensions more consistent with federated extensions - dependencies are shared by default.
This handles the case where one extension says a shared package should not be bundled, but another says it should be bundled.
Since index.out.js was directly importing compiled extensions, webpack was loading the dependencies before index.out.js was initialized. Since federated extensions were loaded in index.out.js, they did not get a chance to provide packages for compiled extensions. This was a problem if you had a compiled extension that depended on a package a federated extension would provide (for example, a custom ipywidgets extension as a compiled extension, with the ipywidget manager as a federated extension).

This loads all federated modules and initializes their containers in the bootstrap phase, making their packages available to the system, before loading the index.out.js file that actually uses them.
@jasongrout
Copy link
Contributor Author

jasongrout commented Nov 11, 2020

A few tests are failing because we have a new file that will appear in staging when we make a release, but since it's not there now, the python commands to copy it over to share/jupyter/lab/staging are failing.

To test this:

conda create -y -n jlab-9289 python nodejs
conda activate jlab-9289

pip install --pre jupyterlab==3.0.0rc7

cd $(mktemp -d)
git clone https://github.com/jasongrout/provider.git
git clone https://github.com/jasongrout/consumer.git

pushd provider
# makes and installs a federated extension
pip install .
# build so we can install in consumer
jlpm install
jlpm run build
popd

pushd consumer
jlpm add `realpath ../provider` # or manually edit package.json to give the absolute path to provider
jupyter labextension install .
popd

Check things with jupyter labextension list.

Now do jupyter lab and notice that there are errors in the console:

Plugin 'consumer' failed to activate.
Error: No provider for: provider:plugin.

and not the expected logs of JupyterLab extension provider is activated!, JupyterLab extension consumer is activated!, Provider token is PROVIDER_STRING.

Now we apply this PR manually. For the patches below, skip any patches that don't apply.

# patch command.py
pushd [site-packages]/jupyterlab
curl https://patch-diff.githubusercontent.com/raw/jupyterlab/jupyterlab/pull/9310.patch | patch -p2

# patch index.js, bootstrap.js, and webpack.config.js
cd [site-packages]/jupyterlab/staging
curl https://patch-diff.githubusercontent.com/raw/jupyterlab/jupyterlab/pull/9310.patch | patch -p2

popd

jupyter lab clean
jupyter lab build

Now run jupyter lab. No errors, and we get the message from the consume plugin indicating it correctly retrieved the object provided by the provider plugin:

Provider token is PROVIDER_STRING

@jasongrout jasongrout marked this pull request as ready for review November 11, 2020 04:12
@jasongrout jasongrout changed the title Share dependencies of built-in extensions Built-in extensions using federated dependencies Nov 11, 2020
@jtpio pointed out that extensions installed from npm are not added to resolutions, but they are top-level extensions that should be shared.
@jtpio
Copy link
Member

jtpio commented Nov 12, 2020

Thanks Jason!

To test this:

Following these steps locally lead to the same observations 👍

I also tested this PR using the same setup as the one described in #9300 (comment), with an extra token package that exports the token between the consumer and the provider.

In this case, the dependencies of the provider (namely the token package) are correctly added to the shared scope, and the consumer (installed as federated) is able to specify the token package (a dependency of both the consumer and the provider) as sharedPackages to enable sharing.


using the same setup as the one described in #9300 (comment)

For extra context and for those interested, here is the setup with a concrete example:

With this PR, and in particular d17aca7 and 721c70d, dependencies of the extension are also added to the shared scope. Which makes it possible for the consumer package installed as federated to define its sharedPackages.

@jasongrout
Copy link
Contributor Author

jasongrout commented Nov 12, 2020

@jtpio - is that a positive review? If it is, I'll merge.

(or even better, you can merge)

jtpio
jtpio approved these changes Nov 12, 2020
Copy link
Member

@jtpio jtpio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@jtpio jtpio merged commit d27a83f into jupyterlab:master Nov 12, 2020
1 check passed
@github-actions github-actions bot added the status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion. label May 12, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants