voila-dashboards / voila

Voilà turns Jupyter notebooks into standalone web applications
https://voila.readthedocs.io
Other
5.41k stars 503 forks source link

Update jupyter-widgets front-end packages #1393

Closed martinRenou closed 1 year ago

martinRenou commented 1 year ago

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[bot] commented 1 year ago

Binder :point_left: Launch a Binder on branch _martinRenou/voila/update_frontendwidgets

trungleduc commented 1 year ago

we could mimic the sharedPackages mechanism of JupyterLab?

jtpio commented 1 year ago

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

martinRenou commented 1 year ago

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

trungleduc commented 1 year ago

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

https://github.com/voila-dashboards/voila/blob/b38b9b80e3186294bfbdf77ecd1ff6eee245e408/packages/voila/webpack.config.js#L15-L23

martinRenou commented 1 year ago

I'll have a try!

trungleduc commented 1 year ago

It seems more complicated than that, we need something like this

https://github.com/jupyterlab/jupyterlab/blob/f2a9603b787068718529fc8e8bcc397156832887/dev_mode/webpack.config.js#L176-L202

jtpio commented 1 year ago

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

martinRenou commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

Aren't the federated extensions loaded before Voila startup?