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

Separate jupyter widgets manager extension #1394

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

martinRenou
Copy link
Member

@martinRenou martinRenou commented Sep 14, 2023

References

A robust fix for #1392

Code changes

  • Move the Voila jupyter-widgets manager into a separate federated extension. This should have the byproduct to properly share the @jupyter-widgets/base package in the federated extension logic, instead of having Voila ship its own copy of this package.
  • Install voila-specific labextensions under share/jupyter/voila/labextensions instead of share/jupyter/labextensions. This prevents JupyterLab to load our widgets-manager and break the classic @jupyter-widgets/jupyterlab-manager

@martinRenou martinRenou added bug Something isn't working enhancement New feature or request labels Sep 14, 2023
@github-actions
Copy link
Contributor

Binder 👈 Launch a Binder on branch martinRenou/voila/split_manager

@martinRenou martinRenou force-pushed the split_manager branch 2 times, most recently from 1cbc7a8 to 96dbce2 Compare September 14, 2023 10:38
@martinRenou
Copy link
Member Author

@trungleduc that works well!! Tested with jupyterlab_widgets 3.0.9 and 3.0.7.

@martinRenou martinRenou marked this pull request as ready for review September 14, 2023 14:02
packages/widgets-manager/package.json Outdated Show resolved Hide resolved
packages/widgets-manager/schema/plugin.json Outdated Show resolved Hide resolved
@trungleduc
Copy link
Member

Question about the releaser, we will bump both package versions on each release or they can be released separately?

@martinRenou
Copy link
Member Author

By default the releaser will bump both but I guess we can configure that in our bump script if we need.

@martinRenou
Copy link
Member Author

The test failure is legit. This new manager breaks jupyterlab because it says it provides the IJupyterWidgetRegistry token (so it overwrites the one from ipywidgets), but it fails saying we need a VoilaApp. This is all very weak.

@martinRenou
Copy link
Member Author

This new manager breaks jupyterlab because it says it provides the IJupyterWidgetRegistry token (so it overwrites the one from ipywidgets), but it fails saying we need a VoilaApp

Because our extension needs a Voila app, I think we should separate the labextensions specific to Voila from the labextensions for other applications and lab-remixes in the path.

So Voila would look into /share/jupyter/labextensions for classic lab-extensions, and /share/jupyter/voila/labextensions for labextensions very specific to Voila. Opinions @jtpio @trungleduc ?

That way we're not breaking JupyterLab with our very specific extension.

@trungleduc
Copy link
Member

how can JupyterLab activate Voila's widget manager? We checked for the app instance before doing anything

@martinRenou
Copy link
Member Author

JupyterLab sees the Voila's widget manager provides the token IJupyterWidgetRegistry so it tries to activate it and that fails. And it does not fallback to the usual widget manager, the Voila one shadows it.

@martinRenou
Copy link
Member Author

It seems depending on @voila-dashboards/voila in a labextension for Voila breaks the labextension with package @jupyterlab/filebrowser is not in the shared scope so we need to remove it.

I see @trungleduc did the same in https://github.com/voila-dashboards/voila-topbar and need to work with (app as any) instead of importing the type of VoilaApp.

Let's open an issue later for this.

The labextension list command will not show voila-specific labextensions
@martinRenou
Copy link
Member Author

Ready for review

@martinRenou
Copy link
Member Author

As discussed with @trungleduc this will be too much of a change for a patch release. @trungleduc found a simpler fix and will come up with another PR.

Though I'd like that we consider these changes in a 0.6.x release, as scoping labextensions specific to Voila may make sense in some cases.

@trungleduc trungleduc added this to the 1.0.0 milestone Sep 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants