🔥🔧 revert passive manager changes (#764)

* Revert "🛠️ fix/direct import bug (#757)" This reverts commit 95eb9b5. * Revert "passive thebe manager (#756)" This reverts commit 30f569e. * 📘 changeset
jupyter-book · Sep 5, 2024 · 80dfbb9 · 80dfbb9
1 parent 6d825e4
commit 80dfbb9
Show file tree

Hide file tree

Showing 19 changed files with 1,109 additions and 643 deletions.
diff --git a/.changeset/metal-eyes-count.md b/.changeset/metal-eyes-count.md
@@ -0,0 +1,7 @@
+---
+'demo-react': patch
+'thebe-react': patch
+'thebe-core': patch
+---
+
+Reverts previous erroneous breaking changes in a patch release
diff --git a/apps/demo-react/package.json b/apps/demo-react/package.json
@@ -10,7 +10,6 @@
     "@types/node": "^16.18.14",
     "@types/react": "^18.0.28",
     "@types/react-dom": "^18.0.11",
-    "classnames": "^2.5.1",
     "localforage": "^1.10.0",
     "match-sorter": "^6.3.1",
     "react": "^18.2.0",

diff --git a/apps/demo-react/public/widget-test.ipynb b/apps/demo-react/public/widget-test.ipynb
diff --git a/apps/demo-react/src/NotebookPage.tsx b/apps/demo-react/src/NotebookPage.tsx
@@ -1,34 +1,23 @@
-import {
-  ThebeSessionProvider,
-  ThebeRenderMimeRegistryProvider,
-  useThebeServer,
-  useThebeLoader,
-} from 'thebe-react';
+import { ThebeSessionProvider, ThebeRenderMimeRegistryProvider, useThebeServer } from 'thebe-react';
 import { ConnectionStatusTray } from './ConnectionStatusTray';
 import { ConnectionErrorTray } from './ConnectionErrorTray';
 import { NotebookStatusTray } from './NotebookStatusTray';
 import { NotebookErrorTray } from './NotebookErrorTray';
 import { AdminPanel } from './AdminPanel';
 
 export function NotebookPage({ children }: React.PropsWithChildren) {
-  const { core } = useThebeLoader();
-  const { connecting, config, ready, error } = useThebeServer();
-
-  if (!core) return null;
+  const { connecting, ready, config, error } = useThebeServer();
 
+  if (!connecting && !ready && !error) return null;
   return (
     <ThebeRenderMimeRegistryProvider>
       <ThebeSessionProvider start path={config?.kernels.path}>
         <>
-          {(connecting || ready || error) && (
-            <>
-              <ConnectionStatusTray />
-              <ConnectionErrorTray />
-              <NotebookStatusTray />
-              <NotebookErrorTray />
-              <AdminPanel />
-            </>
-          )}
+          <ConnectionStatusTray />
+          <ConnectionErrorTray />
+          <NotebookStatusTray />
+          <NotebookErrorTray />
+          <AdminPanel />
           {children}
         </>
       </ThebeSessionProvider>

diff --git a/apps/demo-react/src/WidgetsPage.tsx b/apps/demo-react/src/WidgetsPage.tsx
@@ -1,11 +1,10 @@
 import { useNotebook } from 'thebe-react';
 import JupyterOutputDecoration from './JupyterOutputDecoration';
 import { useParams } from 'react-router-dom';
-import classNames from 'classnames';
 
 export function WidgetsPage() {
   const { notebookName } = useParams<{ notebookName: string }>();
-  const { ready, executing, executeAll, cellRefs, cellIds, errors } = useNotebook(
+  const { ready, executing, executeAll, cellRefs, cellIds } = useNotebook(
     notebookName ?? 'widget-test',
     async (n) => {
       const url = `/${n}.ipynb`;
@@ -28,22 +27,12 @@ export function WidgetsPage() {
       <h4 className="text-sm">
         notebook: <code>{notebookName}.ipynb</code>
       </h4>
-      <div
-        className={classNames(
-          'inline-block px-4 py-2 mt-3 text-sm font-bold text-white rounded-full',
-          { 'bg-gray-500': !ready, 'bg-green-500': ready },
-        )}
-      >
+      <div className="inline-block px-4 py-2 mt-3 text-sm font-bold text-white bg-green-500 rounded-full">
         {ready ? 'ready' : 'not ready'}
       </div>
       <div className="mt-4">
         {!executing && (
-          <button
-            className={classNames('button', {
-              'text-gray-400 bg-gray-100 border-gray-300 cursor-not-allowed': !ready || errors,
-            })}
-            onClick={clickExecute}
-          >
+          <button className="button" onClick={clickExecute}>
             run all
           </button>
         )}

diff --git a/package-lock.json b/package-lock.json
diff --git a/packages/core/src/cell.ts b/packages/core/src/cell.ts
@@ -10,31 +10,32 @@ import { ensureString, shortId } from './utils';
 
 class ThebeCodeCell extends PassiveCellRenderer implements IThebeCell {
   kind: CellKind;
-  notebookId: string;
   source: string;
   metadata: JsonObject;
   session?: ThebeSession;
   executionCount: number | null;
+  protected initialOutputs: IOutput[];
+  readonly notebookId: string;
   protected busy: boolean;
   protected events: EventEmitter;
 
   constructor(
     id: string,
     notebookId: string,
     source: string,
-    outputs: IOutput[],
     config: Config,
     metadata: JsonObject,
     rendermime: IRenderMimeRegistry,
   ) {
-    super(id, outputs, rendermime);
+    super(id, rendermime);
     this.kind = 'code';
     this.events = new EventEmitter(id, config, EventSubject.cell, this);
     this.notebookId = notebookId;
     this.source = source;
     this.metadata = metadata;
     this.busy = false;
     this.executionCount = null;
+    this.initialOutputs = [];
     console.debug('thebe:cell constructor', this);
   }
 
@@ -48,7 +49,6 @@ class ThebeCodeCell extends PassiveCellRenderer implements IThebeCell {
       icc.id ?? shortId(),
       notebookId,
       ensureString(icc.source),
-      icc.outputs ?? [],
       config,
       icc.metadata,
       rendermime,

diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts
@@ -11,7 +11,6 @@ export * from './thebe/api';
 export * from './thebe/entrypoint';
 export * from './utils';
 export * from './manager';
-export * from './passiveManager';
 export * from './rendermime';
 export * from './types';
 export * from './config';

diff --git a/packages/core/src/manager.ts b/packages/core/src/manager.ts
@@ -1,23 +1,20 @@
 import type { IRenderMimeRegistry } from '@jupyterlab/rendermime';
 import type { Widget } from '@lumino/widgets';
+
 import * as LuminoWidget from '@lumino/widgets';
 import { MessageLoop } from '@lumino/messaging';
+
 import { KernelWidgetManager, WidgetRenderer, output } from '@jupyter-widgets/jupyterlab-manager';
+
+export const WIDGET_MIMETYPE = 'application/vnd.jupyter.widget-view+json';
+
 import * as base from '@jupyter-widgets/base';
 import * as controls from '@jupyter-widgets/controls';
 import { shortId } from './utils';
 import { RequireJsLoader } from './requireJsLoader';
 import { requireLoader } from './loader';
 import type { Kernel } from '@jupyterlab/services';
 
-export const WIDGET_STATE_MIMETYPE = 'application/vnd.jupyter.widget-state+json';
-export const WIDGET_VIEW_MIMETYPE = 'application/vnd.jupyter.widget-view+json';
-
-/**
- * @deprecated use WIDGET_VIEW_MIMETYPE
- */
-export const WIDGET_MIMETYPE = WIDGET_VIEW_MIMETYPE;
-
 /**
  * A Widget Manager class for Thebe using the context-free KernelWidgetManager from
  * the JupyterLab  Manager and inspierd by the implementation in Voila here:
@@ -35,17 +32,25 @@ export class ThebeManager extends KernelWidgetManager {
     /** ensure this registry always gets the widget renderer.
      * This is essential for cases where widgets are rendered heirarchically
      */
+    this.addWidgetFactories();
+
+    this._registerWidgets();
+    this._loader = new RequireJsLoader();
+  }
+
+  addWidgetFactories() {
     this.rendermime.addFactory(
       {
         safe: false,
-        mimeTypes: [WIDGET_VIEW_MIMETYPE],
+        mimeTypes: [WIDGET_MIMETYPE],
         createRenderer: (options) => new WidgetRenderer(options, this as any),
       },
       1,
     );
+  }
 
-    this._registerWidgets();
-    this._loader = new RequireJsLoader();
+  removeWidgetFactories() {
+    this.rendermime.removeMimeType(WIDGET_MIMETYPE);
   }
 
   /**

diff --git a/packages/core/src/markdown.ts b/packages/core/src/markdown.ts
@@ -5,7 +5,6 @@ import PassiveCellRenderer from './passive';
 import type ThebeSession from './session';
 import type { CellKind, IThebeCell, IThebeCellExecuteReturn, JsonObject } from './types';
 import { ensureString, shortId } from './utils';
-import type ThebeNotebook from './notebook';
 
 /**
  * A Thebe cell that is exepected to contain markdown (or raw) source.
@@ -29,7 +28,7 @@ export default class ThebeMarkdownCell extends PassiveCellRenderer implements IT
     metadata: JsonObject,
     rendermime: IRenderMimeRegistry,
   ) {
-    super(id, [], rendermime);
+    super(id, rendermime);
     this.kind = 'markdown';
     this.id = id;
     this.notebookId = notebookId;

diff --git a/packages/core/src/notebook.ts b/packages/core/src/notebook.ts
@@ -15,32 +15,19 @@ export interface CodeBlock {
   [x: string]: any;
 }
 
-function coerceToObject(maybe: any): Record<string, any> {
-  if (typeof maybe === 'object') return maybe;
-  if (Array.isArray(maybe)) return Object.fromEntries(maybe.map((v, k) => [k, v]));
-  return {};
-}
-
 class ThebeNotebook {
   readonly id: string;
   readonly rendermime: IRenderMimeRegistry;
   cells: IThebeCell[];
   metadata: INotebookMetadata;
-  widgetState: Record<string, any>;
   session?: ThebeSession;
   protected events: EventEmitter;
 
-  constructor(
-    id: string,
-    config: Config,
-    rendermime: IRenderMimeRegistry,
-    metadata?: INotebookMetadata,
-  ) {
+  constructor(id: string, config: Config, rendermime: IRenderMimeRegistry) {
     this.id = id;
     this.events = new EventEmitter(id, config, EventSubject.notebook, this);
     this.cells = [];
-    this.metadata = metadata ?? {};
-    this.widgetState = coerceToObject(metadata?.widgets);
+    this.metadata = {};
     this.rendermime = rendermime;
     console.debug('thebe:notebook constructor', this);
   }
@@ -50,15 +37,7 @@ class ThebeNotebook {
     const notebook = new ThebeNotebook(id, config, rendermime);
     notebook.cells = blocks.map((c) => {
       const metadata = {};
-      const cell = new ThebeCodeCell(
-        c.id,
-        notebook.id,
-        c.source,
-        c.outputs ?? [],
-        config,
-        metadata,
-        notebook.rendermime,
-      );
+      const cell = new ThebeCodeCell(c.id, id, c.source, config, metadata, notebook.rendermime);
       console.debug(`thebe:notebook:fromCodeBlocks Initializing cell ${c.id}`);
       return cell;
     });
@@ -92,9 +71,6 @@ class ThebeNotebook {
     return p;
   }
 
-  /**
-  @deprecated
-   */
   get widgets() {
     return this.findCells('widget') ?? [];
   }

diff --git a/packages/core/src/passive.ts b/packages/core/src/passive.ts
@@ -10,25 +10,17 @@ import { MessageLoop } from '@lumino/messaging';
 class PassiveCellRenderer implements IPassiveCell {
   readonly id: string;
   readonly rendermime: IRenderMimeRegistry;
-  initialOutputs: nbformat.IOutput[];
-
   protected model: OutputAreaModel;
   protected area: OutputArea;
 
-  constructor(
-    id: string,
-    initialOutputs?: nbformat.IOutput[],
-    rendermime?: IRenderMimeRegistry,
-    mathjax?: MathjaxOptions,
-  ) {
+  constructor(id: string, rendermime?: IRenderMimeRegistry, mathjax?: MathjaxOptions) {
     this.id = id;
     this.rendermime = rendermime ?? makeRenderMimeRegistry(mathjax ?? makeMathjaxOptions());
     this.model = new OutputAreaModel({ trusted: true });
     this.area = new OutputArea({
       model: this.model,
       rendermime: this.rendermime,
     });
-    this.initialOutputs = initialOutputs ?? [];
   }
 
   /**
@@ -42,10 +34,7 @@ class PassiveCellRenderer implements IPassiveCell {
     return this.area.isAttached;
   }
 
-  attachToDOM(
-    el?: HTMLElement,
-    opts: { strict?: boolean; appendExisting?: boolean } = { strict: false, appendExisting: true },
-  ) {
+  attachToDOM(el?: HTMLElement, strict = false) {
     if (!this.area || !el) {
       console.error(
         `thebe:renderer:attachToDOM - could not attach to DOM - area: ${this.area}, el: ${el}`,
@@ -55,11 +44,11 @@ class PassiveCellRenderer implements IPassiveCell {
     if (this.area.isAttached) {
       // TODO should we detach and reattach?
       console.debug(`thebe:renderer:attachToDOM - already attached`);
-      if (opts.strict) return;
+      if (strict) return;
     } else {
       // if the target element has contents, preserve it but wrap it in our output area
       console.debug(`thebe:renderer:attachToDOM ${this.id} - appending existing contents`);
-      if (opts.appendExisting && el.innerHTML) {
+      if (el.innerHTML) {
         this.area.model.add({
           output_type: 'display_data',
           data: {