fix/tolerant providers (#683)

* 🔧 use correct binder repo url
* 🫴🏼 hooks no longer throw on undefined contexts
* 📕 changeset
* allow custom repoproviderspecs to be added to react server provider
* 🥁don't provide np-ops
* 🙅🏽 throw exception for malformed RepoProviderSpec
* fix/server blatting (#685)
* 📡 add check call for user server
* 👮🏻 prevent reconnect for existing server
* 📕 changeset

.changeset/metal-humans-beg.md

@@ -0,0 +1,5 @@
+---
+'thebe-core': patch
+---
+
+Add check call for user server status

.changeset/soft-countries-invite.md

@@ -0,0 +1,6 @@
+---
+'demo-react': patch
+'thebe-react': patch
+---
+
+ThebeServerProvider will no longer replace an existing server on rerender if that server is ready and has user server URL.

.changeset/witty-zoos-smell.md

@@ -0,0 +1,6 @@
+---
+'demo-react': patch
+'thebe-react': patch
+---
+
+Hooks no longer throw but return empty or uninitialised objects instead, this allows for much more flexibility in how respective providers are rendered.

apps/demo-react/src/App.tsx

@@ -19,7 +19,7 @@ function App() {
         kernelName: 'python',
       },
       binderOptions: {
-        repo: 'curvenote/binder-base',
+        repo: 'executablebooks/thebe-binder-base',
       },
     }),
     [path],

apps/demo-react/src/Connect.tsx

@@ -11,7 +11,7 @@ export function Connect() {
   }, [core, load, loading]);
 
   const clickConnect = () => {
-    if (!core) return;
+    if (!core || !connect) return;
     connect();
   };
 

packages/core/src/server.ts

@@ -79,6 +79,13 @@ class ThebeServer implements ServerRuntime, ServerRestAPI {
     return this.sessionManager?.shutdownAll();
   }
 
+  async check(): Promise<boolean> {
+    const resp = await ThebeServer.status(
+      this.sessionManager?.serverSettings ?? this.config.serverSettings,
+    );
+    return resp.ok;
+  }
+
   dispose() {
     if (this._isDisposed) return;
     if (!this.serviceManager?.isDisposed) this.serviceManager?.dispose();
@@ -318,6 +325,7 @@ class ThebeServer implements ServerRuntime, ServerRestAPI {
         });
 
         return this.sessionManager.ready.then(() => {
+          this.userServerUrl = `${serverSettings.baseUrl}?token=${serverSettings.token}`;
           this.events.triggerStatus({
             status: ServerStatusEvent.ready,
             message: `Re-connected to binder server`,
@@ -438,7 +446,7 @@ class ThebeServer implements ServerRuntime, ServerRestAPI {
     return url;
   }
 
-  static status(serverSettings: Required<ServerSettings>): Promise<void | Response> {
+  static status(serverSettings: ServerSettings) {
     return ServerConnection.makeRequest(
       `${serverSettings.baseUrl}api/status`,
       {},

packages/core/src/url.ts

@@ -118,5 +118,7 @@ export function makeBinderUrls(
   if (!Object.keys(providerMap).includes(provider))
     throw Error(`Unknown provider ${opts.repoProvider}`);
 
+  if (!providerMap[provider].makeUrls) throw Error(`No makeUrls function for ${provider}`);
+
   return providerMap[provider].makeUrls(opts);
 }

packages/react/src/ThebeLoaderProvider.tsx

@@ -118,10 +118,5 @@ export function ThebeBundleLoaderProvider({
 
 export function useThebeLoader() {
   const context = React.useContext(ThebeLoaderContext);
-  if (context === undefined) {
-    throw new Error(
-      'useThebeLoader must be used inside a ThebeLoaderProvider or ThebeBundleLoaderProvider',
-    );
-  }
-  return context;
+  return context ?? { loading: false, load: () => ({}) };
 }

packages/react/src/ThebeRenderMimeRegistryProvider.tsx

@@ -25,8 +25,5 @@ export function ThebeRenderMimeRegistryProvider({ children }: React.PropsWithChi
 
 export function useRenderMimeRegistry() {
   const context = React.useContext(RenderMimeRegistryContext);
-  if (context === undefined) {
-    throw new Error('useRenderMimeRegistry must be used within a ThebeRenderMimeRegistry');
-  }
-  return context.rendermime;
+  return context?.rendermime;
 }

packages/react/src/ThebeServerProvider.tsx

@@ -2,6 +2,7 @@ import React, { useCallback, useContext, useEffect, useMemo, useState } from 're
 import type {
   Config,
   CoreOptions,
+  RepoProviderSpec,
   ThebeEventCb,
   ThebeEventData,
   ThebeEvents,
@@ -13,13 +14,13 @@ type ListenerFn = (data: ThebeEventData) => void;
 
 export const ThebeServerContext = React.createContext<
   | {
+      connecting: boolean;
+      ready: boolean;
       config?: Config;
       events?: ThebeEvents;
       server?: ThebeServer;
-      connecting: boolean;
-      ready: boolean;
-      connect: () => void;
-      disconnect: () => Promise<void>;
+      connect?: () => void;
+      disconnect?: () => Promise<void>;
     }
   | undefined
 >(undefined);
@@ -31,6 +32,7 @@ export function ThebeServerProvider({
   useBinder,
   useJupyterLite,
   customConnectFn,
+  customRepoProviders,
   events,
   children,
 }: React.PropsWithChildren<{
@@ -41,6 +43,7 @@ export function ThebeServerProvider({
   useJupyterLite?: boolean;
   events?: ThebeEvents;
   customConnectFn?: (server: ThebeServer) => Promise<void>;
+  customRepoProviders?: RepoProviderSpec[];
 }>) {
   const { core } = useThebeLoader();
   const [doConnect, setDoConnect] = useState(connect);
@@ -57,17 +60,21 @@ export function ThebeServerProvider({
   );
 
   useEffect(() => {
-    if (!core || !thebeConfig) return;
+    if (!core || !thebeConfig || server) return;
     setServer(new core.ThebeServer(thebeConfig));
-  }, [core, thebeConfig]);
+  }, [core, thebeConfig, server]);
 
   // Once the core is loaded, connect to a server
+  // TODO: this should be an action not a side effect
   useEffect(() => {
     if (!core || !thebeConfig) return; // TODO is there a better way to keep typescript happy here?
     if (!server || !doConnect) return;
+    // do not reconnect if already connected!
+    if (server.isReady && server.userServerUrl) return;
+    // TODO is the user server really still alive? this would be an async call to server.check
     setConnecting(true);
     if (customConnectFn) customConnectFn(server);
-    else if (useBinder) server.connectToServerViaBinder();
+    else if (useBinder) server.connectToServerViaBinder(customRepoProviders);
     else if (useJupyterLite)
       server.connectToJupyterLiteServer({
         litePluginSettings: {
@@ -140,13 +147,15 @@ export function useDisposeThebeServer() {
 }
 
 export function useThebeServer() {
+  const thebe = useThebeLoader();
+  const { core } = thebe ?? {};
+
   const serverContext = useContext(ThebeServerContext);
-  if (serverContext === undefined) {
-    throw new Error('useThebeServer must be used inside a ThebeServerProvider');
-  }
-  const { config, events, server, connecting, ready, connect, disconnect } = serverContext;
+  const { config, events, server, connecting, ready, connect, disconnect } = serverContext ?? {
+    ready: false,
+    connecting: false,
+  };
 
-  const { core } = useThebeLoader();
   const [error, setError] = useState<string | undefined>(); // TODO how to handle errors better via the provider
   const [eventCallbacks, setEventCallbacks] = useState<ThebeEventCb[]>([]);
 
@@ -180,16 +189,18 @@ export function useThebeServer() {
     setEventCallbacks([]);
   }, [config, server]);
 
-  return {
-    config,
-    events,
-    server,
-    connecting,
-    ready,
-    error,
-    connect,
-    disconnect,
-    subscribe,
-    unsubAll,
-  };
+  return serverContext
+    ? {
+        config,
+        events,
+        server,
+        connecting,
+        ready,
+        error,
+        connect,
+        disconnect,
+        subscribe,
+        unsubAll,
+      }
+    : { connecting: false, ready: false };
 }

packages/react/src/ThebeSessionProvider.tsx

@@ -4,13 +4,13 @@ import { useThebeServer } from './ThebeServerProvider';
 import { useRenderMimeRegistry } from './ThebeRenderMimeRegistryProvider';
 
 interface ThebeSessionContextData {
-  path: string;
+  path?: string;
   session?: ThebeSession;
+  error?: string;
   starting: boolean;
   ready: boolean;
-  error?: string;
-  start: () => Promise<void>;
-  shutdown: () => Promise<void>;
+  start?: () => Promise<void>;
+  shutdown?: () => Promise<void>;
 }
 
 export const ThebeSessionContext = React.createContext<ThebeSessionContextData | undefined>(
@@ -36,6 +36,7 @@ export function ThebeSessionProvider({
   const [error, setError] = useState<string | undefined>();
 
   const startSession = () => {
+    if (!rendermime) throw new Error('ThebeSessionProvider requires a RenderMimeRegistryProvider');
     setStarting(true);
     server
       ?.startNewSession(rendermime, { ...config?.kernels, path })
@@ -103,8 +104,5 @@ export function ThebeSessionProvider({
 
 export function useThebeSession(): ThebeSessionContextData {
   const sessionContext = useContext(ThebeSessionContext);
-  if (sessionContext === undefined) {
-    throw new Error('useThebeSession must be used inside a ThebeSessionProvider');
-  }
-  return sessionContext;
+  return sessionContext ?? { starting: false, ready: false };
 }

packages/react/src/hooks/notebook.ts

@@ -5,7 +5,6 @@ import { useThebeLoader } from '../ThebeLoaderProvider';
 import type { INotebookContent } from '@jupyterlab/nbformat';
 import { useThebeSession } from '../ThebeSessionProvider';
 import { useRenderMimeRegistry } from '../ThebeRenderMimeRegistryProvider';
-import { render } from 'react-dom';
 
 export interface NotebookExecuteOptions {
   stopOnError?: boolean;
@@ -124,6 +123,8 @@ export function useNotebook(
   const rendermime = useRenderMimeRegistry();
   const [loading, setLoading] = useState<boolean>(false);
 
+  if (!rendermime) throw new Error('ThebeSessionProvider requires a RenderMimeRegistryProvider');
+
   const {
     ready,
     attached,
@@ -199,6 +200,7 @@ export function useNotebookFromSource(sourceCode: string[], opts = { refsForWidg
   const { config } = useThebeConfig();
   const rendermime = useRenderMimeRegistry();
   const [loading, setLoading] = useState(false);
+  if (!rendermime) throw new Error('ThebeSessionProvider requires a RenderMimeRegistryProvider');
   const {
     ready,
     attached,
@@ -262,6 +264,7 @@ export function useNotebookfromSourceLegacy(sourceCode: string[]) {
   const { core } = useThebeLoader();
   const { config } = useThebeConfig();
   const rendermime = useRenderMimeRegistry();
+  if (!rendermime) throw new Error('ThebeSessionProvider requires a RenderMimeRegistryProvider');
 
   const [busy, setBusy] = useState<boolean>(false);
   const [notebook, setNotebook] = useState<ThebeNotebook | undefined>();