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

Update jupyter-widgets front-end packages #1393

Merged

Conversation

martinRenou
Copy link
Member

@martinRenou martinRenou commented Sep 13, 2023

Would fix #1392 ?

I am very surprised this breaks. I wonder if we don't have a broken setup where we don't really use the @jupyter-widgets/base and controls ipywidgets provide, but we provide a copy of it in the page. (JupyterLab does not have this problem)

Would there be a way to properly use them as shared packages? probably a question for @jtpio ?

@github-actions
Copy link
Contributor

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

@martinRenou martinRenou added the bug Something isn't working label Sep 13, 2023
@martinRenou martinRenou merged commit b38b9b8 into voila-dashboards:main Sep 13, 2023
27 of 28 checks passed
@martinRenou martinRenou deleted the update_frontend_widgets branch September 13, 2023 13:30
@trungleduc
Copy link
Member

we could mimic the sharedPackages mechanism of JupyterLab?

@jtpio
Copy link
Member

jtpio commented Sep 13, 2023

Yeah I also thought we were already using the widgets prebuilt extension for 0.5.0.

@martinRenou
Copy link
Member Author

We are, but we also have our own jupyterlab-manager, which I guess means it can conflicts with what ipywidgets provides?

@trungleduc
Copy link
Member

Adding "jupyter-widgets package check here might be enough?

const names = Object.keys(data.dependencies).filter((name) => {
if (name === 'style-mod') {
return false;
}
const packageData = require(path.join(name, 'package.json'));
return packageData.jupyterlab !== undefined;
});

@martinRenou
Copy link
Member Author

I'll have a try!

@trungleduc
Copy link
Member

trungleduc commented Sep 13, 2023

@jtpio
Copy link
Member

jtpio commented Sep 13, 2023

You can also have a look at the notebook 7 webpack config for inspiration.

@martinRenou
Copy link
Member Author

The issue is actually that the jupyterlab_widgets lab-extensions builds against its own version of @jupyter-widgets/base, so the Token for jupyter.extensions.jupyterWidgetRegistry is different between Voila and ipywidgets

So I think we're in a different situation than both JupyterLab and Notebook 7, because they don't mess up with widgets tokens like we do.

We need to have @jupyter-widgets/base be properly shared between Voila and the jupyterlab_widgets lab-extension.

I tried the following:

---
 packages/voila/src/sharedscope.ts |  1 +
 packages/voila/webpack.config.js  | 20 +++++++++++++++++---
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/packages/voila/src/sharedscope.ts b/packages/voila/src/sharedscope.ts
index 26ac315..c428085 100644
--- a/packages/voila/src/sharedscope.ts
+++ b/packages/voila/src/sharedscope.ts
@@ -13,3 +13,4 @@ import '@lumino/signaling';
 import '@lumino/virtualdom';
 import '@lumino/widgets';
 import 'react-dom';
+import '@jupyter-widgets/base';
diff --git a/packages/voila/webpack.config.js b/packages/voila/webpack.config.js
index 95e3701..b52cbf0 100644
--- a/packages/voila/webpack.config.js
+++ b/packages/voila/webpack.config.js
@@ -21,6 +21,22 @@ const names = Object.keys(data.dependencies).filter((name) => {
   return packageData.jupyterlab !== undefined;
 });
 
+const shared = {
+  ...data.dependencies
+};
+shared["@jupyter-widgets/base"] = {
+  requiredVersion: shared["@jupyter-widgets/base"],
+  singleton: true,
+  // import: false,
+};
+shared["@jupyter-widgets/jupyterlab-manager"] = {
+  requiredVersion: shared["@jupyter-widgets/jupyterlab-manager"],
+  singleton: true,
+  // import: false,
+};
+
+console.log('--- SHARED', shared);
+
 // Ensure a clear build directory.
 const buildDir = path.resolve(__dirname, 'build');
 if (fs.existsSync(buildDir)) {
@@ -95,9 +111,7 @@ module.exports = [
           name: ['_JUPYTERLAB', 'CORE_LIBRARY_FEDERATION']
         },
         name: 'CORE_FEDERATION',
-        shared: {
-          ...data.dependencies
-        }
+        shared
       })
     ]
   }),
-- 
2.41.0

But it does not seem sufficient (setting import: false would force Voila to not bundle, and Voila would fail at importing @jupyter-widgets/base).

I wonder if the jupyterlab_widgets lab-extension should not also set @jupyter-widgets/base as being shared here https://github.com/jupyter-widgets/ipywidgets/blob/main/python/jupyterlab_widgets/package.json#L91 ? Does it make sense?

@jtpio
Copy link
Member

jtpio commented Sep 13, 2023

I wonder if the jupyterlab_widgets lab-extension should not also set @jupyter-widgets/base as being shared here https://github.com/jupyter-widgets/ipywidgets/blob/main/python/jupyterlab_widgets/package.json#L91 ?

As shared but not bundled? If not bundled, then another extension would have to bring @jupyter-widgets/base so it's available on the page.

@martinRenou
Copy link
Member Author

Yeah I think it should bundle it, otherwise we would break jupyterlab_widgets support in JupyterLab and Notebook 7.

But that's what confuses me in the case of Voila. Who should be responsible for bundling @jupyter-widgets/base?
If it's jupyterlab_widgets, then how is Voila supposed to load it? Voila needs to create its manager before loading the jupyterlab_widgets extension, so it needs @jupyter-widgets/base in the scope, but we have it only when the jupyterlab_widgets is loaded. It's a chicken and egg situation here.

@trungleduc
Copy link
Member

Aren't the federated extensions loaded before Voila startup?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Notebook widgets suddenly broke
3 participants