remove name from kernelOptions (#677)

* remove `name` from `kernelOptions` * 🔧 updated demo apps * 🧹 lint
jupyter-book · Aug 21, 2023 · 1b2363d · 1b2363d
1 parent ecb07a5
commit 1b2363d
Show file tree

Hide file tree

Showing 10 changed files with 1,436 additions and 23 deletions.
diff --git a/apps/demo-react/public/path-test.ipynb b/apps/demo-react/public/path-test.ipynb
diff --git a/apps/demo-react/src/App.tsx b/apps/demo-react/src/App.tsx
@@ -1,24 +1,28 @@
 import { useMemo, useState } from 'react';
 import { ThebeBundleLoaderProvider, ThebeServerProvider } from 'thebe-react';
-import { NavLink, Outlet } from 'react-router-dom';
+import { NavLink, Outlet, useLocation } from 'react-router-dom';
 import './App.css';
 import { Connect } from './Connect';
 import { ServerMode, ServerModeType } from './ServerMode';
 import { ThebeStatusTray } from './ThebeStatusTray';
 
 function App() {
   const [mode, setMode] = useState<ServerModeType>('local');
+  const location = useLocation();
+
+  const path = location.pathname.endsWith('path-test') ? '/notebooks' : '/';
 
   const options = useMemo(
     () => ({
       kernelOptions: {
-        name: 'Python 3',
+        path,
+        kernelName: 'python',
       },
       binderOptions: {
         repo: 'curvenote/binder-base',
       },
     }),
-    [],
+    [path],
   );
 
   return (
@@ -55,7 +59,7 @@ function App() {
         </ThebeServerProvider>
       </ThebeBundleLoaderProvider>
 
-      <div className="fixed top-2 right-1 text-xs">
+      <div className="fixed text-xs top-2 right-1">
         Server icon by Ralf Schmitzer from{' '}
         <a
           href="https://thenounproject.com/browse/icons/term/server/"

diff --git a/apps/demo-react/src/NotebookPage.tsx b/apps/demo-react/src/NotebookPage.tsx
@@ -1,12 +1,12 @@
 import { ThebeSessionProvider, ThebeRenderMimeRegistryProvider, useThebeServer } from 'thebe-react';
 
-export function NotebookPage({ name, children }: React.PropsWithChildren<{ name: string }>) {
-  const { ready } = useThebeServer();
+export function NotebookPage({ children }: React.PropsWithChildren) {
+  const { ready, config } = useThebeServer();
 
   if (!ready) return null;
   return (
     <ThebeRenderMimeRegistryProvider>
-      <ThebeSessionProvider start name={name}>
+      <ThebeSessionProvider start path={config?.kernels.path}>
         {children}
       </ThebeSessionProvider>
     </ThebeRenderMimeRegistryProvider>

diff --git a/apps/demo-react/src/index.tsx b/apps/demo-react/src/index.tsx
@@ -20,7 +20,7 @@ const router = createBrowserRouter([
       {
         path: 'nb/:notebookName',
         element: (
-          <NotebookPage name="widgets">
+          <NotebookPage>
             <WidgetsPage />
           </NotebookPage>
         ),

diff --git a/packages/core/src/options.ts b/packages/core/src/options.ts
@@ -33,7 +33,6 @@ export function makeKernelOptions(opts: KernelOptions): Required<KernelOptions>
   return {
     path: opts.path ?? '/',
     kernelName: opts.kernelName ?? 'python',
-    name: opts.name ?? opts.kernelName ?? 'python',
   };
 }
 

diff --git a/packages/core/src/server.ts b/packages/core/src/server.ts
@@ -90,10 +90,19 @@ class ThebeServer implements ServerRuntime, ServerRestAPI {
       throw Error('Requesting session from a server, with no SessionManager available');
     }
 
-    // TODO can we defer connection setup, return the session immediately and then have a ready signal?
+    // name is assumed to be a non empty string but is otherwise note required
+    // if a notebook name has been supplied on the path, use that otherwise use a default
+    // https://jupyterlab.readthedocs.io/en/3.4.x/api/modules/services.session.html#isessionoptions
+    const path = kernelOptions?.path ?? this.config.kernels.path;
+    let name = 'thebe.ipynb';
+    const match = path.match(/\/*([a-zA-Z0-9]+.ipynb)$/);
+    if (match) {
+      name = match[1];
+    }
+
     const connection = await this.sessionManager?.startNew({
-      name: kernelOptions?.name ?? kernelOptions?.kernelName ?? this.config.kernels.name,
-      path: kernelOptions?.path ?? this.config.kernels.path,
+      name,
+      path,
       type: 'notebook',
       kernel: {
         name: kernelOptions?.kernelName ?? this.config.kernels.kernelName,

diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts
@@ -69,7 +69,6 @@ export interface ServerSettings {
 }
 
 export interface KernelOptions {
-  name?: string;
   kernelName?: string;
   path?: string;
 }

diff --git a/packages/core/tests/config.spec.ts b/packages/core/tests/config.spec.ts
@@ -25,7 +25,6 @@ describe('config', () => {
     test('kernels', () => {
       expect(config.kernels).toEqual({
         path: '/',
-        name: 'python',
         kernelName: 'python',
       });
     });

diff --git a/packages/core/tests/options.spec.ts b/packages/core/tests/options.spec.ts
@@ -60,20 +60,17 @@ describe('options', () => {
     test('defaults', () => {
       expect(makeKernelOptions({})).toEqual({
         path: '/',
-        name: 'python',
         kernelName: 'python',
       });
     });
     test('overrides', () => {
       expect(
         makeKernelOptions({
           path: '/notebooks',
-          name: 'julia',
           kernelName: 'ijpl1',
         }),
       ).toEqual({
         path: '/notebooks',
-        name: 'julia',
         kernelName: 'ijpl1',
       });
     });
@@ -85,7 +82,6 @@ describe('options', () => {
         }),
       ).toEqual({
         path: '/notebooks',
-        name: 'ijpl1',
         kernelName: 'ijpl1',
       });
     });

diff --git a/packages/react/src/ThebeSessionProvider.tsx b/packages/react/src/ThebeSessionProvider.tsx
@@ -4,7 +4,7 @@ import { useThebeServer } from './ThebeServerProvider';
 import { useRenderMimeRegistry } from './ThebeRenderMimeRegistryProvider';
 
 interface ThebeSessionContextData {
-  name: string;
+  path: string;
   session?: ThebeSession;
   starting: boolean;
   ready: boolean;
@@ -19,12 +19,12 @@ export const ThebeSessionContext = React.createContext<ThebeSessionContextData |
 
 export function ThebeSessionProvider({
   start = true,
-  name = 'default',
+  path = '/thebe.ipynb',
   shutdownOnUnmount = false,
   children,
 }: React.PropsWithChildren<{
   start?: boolean;
-  name?: string;
+  path?: string;
   shutdownOnUnmount?: boolean;
 }>) {
   const { config, server, ready: serverReady } = useThebeServer();
@@ -38,7 +38,7 @@ export function ThebeSessionProvider({
   const startSession = () => {
     setStarting(true);
     server
-      ?.startNewSession(rendermime, { ...config?.kernels, name, path: name })
+      ?.startNewSession(rendermime, { ...config?.kernels, path })
       .then((sesh: ThebeSession | null) => {
         setStarting(false);
         if (sesh == null) {
@@ -74,7 +74,7 @@ export function ThebeSessionProvider({
   return (
     <ThebeSessionContext.Provider
       value={{
-        name,
+        path,
         starting,
         ready,
         session,