diff --git a/.changeset/metal-humans-beg.md b/.changeset/metal-humans-beg.md new file mode 100644 index 00000000..17d0eafc --- /dev/null +++ b/.changeset/metal-humans-beg.md @@ -0,0 +1,5 @@ +--- +'thebe-core': patch +--- + +Add check call for user server status diff --git a/.changeset/soft-countries-invite.md b/.changeset/soft-countries-invite.md new file mode 100644 index 00000000..88dbcbf0 --- /dev/null +++ b/.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. diff --git a/.changeset/witty-zoos-smell.md b/.changeset/witty-zoos-smell.md new file mode 100644 index 00000000..4384f2ef --- /dev/null +++ b/.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. diff --git a/apps/demo-react/src/App.tsx b/apps/demo-react/src/App.tsx index 3e675737..19c7fb61 100644 --- a/apps/demo-react/src/App.tsx +++ b/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], diff --git a/apps/demo-react/src/Connect.tsx b/apps/demo-react/src/Connect.tsx index 561e25eb..5ff0bdd4 100644 --- a/apps/demo-react/src/Connect.tsx +++ b/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(); }; diff --git a/packages/core/src/server.ts b/packages/core/src/server.ts index 125f4937..00490d68 100644 --- a/packages/core/src/server.ts +++ b/packages/core/src/server.ts @@ -79,6 +79,13 @@ class ThebeServer implements ServerRuntime, ServerRestAPI { return this.sessionManager?.shutdownAll(); } + async check(): Promise { + 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): Promise { + static status(serverSettings: ServerSettings) { return ServerConnection.makeRequest( `${serverSettings.baseUrl}api/status`, {}, diff --git a/packages/core/src/url.ts b/packages/core/src/url.ts index cbfe6a41..b958a2cd 100644 --- a/packages/core/src/url.ts +++ b/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); } diff --git a/packages/react/src/ThebeLoaderProvider.tsx b/packages/react/src/ThebeLoaderProvider.tsx index 3a87e62e..74ec6bee 100644 --- a/packages/react/src/ThebeLoaderProvider.tsx +++ b/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: () => ({}) }; } diff --git a/packages/react/src/ThebeRenderMimeRegistryProvider.tsx b/packages/react/src/ThebeRenderMimeRegistryProvider.tsx index d574f782..76e6a0b4 100644 --- a/packages/react/src/ThebeRenderMimeRegistryProvider.tsx +++ b/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; } diff --git a/packages/react/src/ThebeServerProvider.tsx b/packages/react/src/ThebeServerProvider.tsx index d8328736..e9ebea2d 100644 --- a/packages/react/src/ThebeServerProvider.tsx +++ b/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; + connect?: () => void; + disconnect?: () => Promise; } | 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; + 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(); // TODO how to handle errors better via the provider const [eventCallbacks, setEventCallbacks] = useState([]); @@ -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 }; } diff --git a/packages/react/src/ThebeSessionProvider.tsx b/packages/react/src/ThebeSessionProvider.tsx index 2d8dd221..ee2c082a 100644 --- a/packages/react/src/ThebeSessionProvider.tsx +++ b/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; - shutdown: () => Promise; + start?: () => Promise; + shutdown?: () => Promise; } export const ThebeSessionContext = React.createContext( @@ -36,6 +36,7 @@ export function ThebeSessionProvider({ const [error, setError] = useState(); 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 }; } diff --git a/packages/react/src/hooks/notebook.ts b/packages/react/src/hooks/notebook.ts index 74064513..8c9f8246 100644 --- a/packages/react/src/hooks/notebook.ts +++ b/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(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(false); const [notebook, setNotebook] = useState();