fix: Add ESLint rules for type safety and fix violations

jest.config.js

@@ -4,13 +4,15 @@ const esModules = [
   '@codemirror',
   '@jupyter',
   '@microsoft',
+  '@deepnote',
   'exenv-es6',
   'lib0',
   'nanoid',
   'vscode-ws-jsonrpc',
   'y-protocols',
   'y-websocket',
-  'yjs'
+  'yjs',
+  'yaml'
 ].join('|');
 
 const baseConfig = jestJupyterLab(__dirname);

package.json

@@ -161,9 +161,12 @@
                     "args": "none"
                 }
             ],
-            "@typescript-eslint/no-explicit-any": "off",
+            "@typescript-eslint/no-explicit-any": "error",
             "@typescript-eslint/no-namespace": "off",
             "@typescript-eslint/no-use-before-define": "off",
+            "@typescript-eslint/no-floating-promises": "error",
+            "@typescript-eslint/no-non-null-assertion": "error",
+            "@typescript-eslint/prefer-nullish-coalescing": "error",
             "@typescript-eslint/quotes": [
                 "error",
                 "single",

src/__tests__/NotebookPicker.spec.ts

@@ -1,111 +1,269 @@
 // Copyright (c) Deepnote
 // Distributed under the terms of the Modified BSD License.
 
-import { NotebookPicker } from '../../src/components/NotebookPicker';
+import type { INotebookModel, NotebookPanel } from '@jupyterlab/notebook';
 import { framePromise } from '@jupyterlab/testing';
-import { NotebookPanel } from '@jupyterlab/notebook';
-import { INotebookModel } from '@jupyterlab/notebook';
+import type { PartialJSONObject } from '@lumino/coreutils';
 import { Widget } from '@lumino/widgets';
-import { simulate } from 'simulate-event';
+import { NotebookPicker } from '../components/NotebookPicker';
 
 describe('NotebookPicker', () => {
-  let panel: NotebookPanel;
-  let model: INotebookModel;
+  let widget: NotebookPicker;
+  let mockNotebookModel: Partial<INotebookModel>;
+  let deepnoteMetadata: PartialJSONObject;
+  let consoleErrorSpy: jest.SpyInstance | null = null;
 
-  beforeEach(async () => {
-    // Mock model + metadata
-    model = {
+  const createMockPanel = (metadata: PartialJSONObject): NotebookPanel => {
+    deepnoteMetadata = metadata;
+
+    mockNotebookModel = {
       fromJSON: jest.fn(),
-      get cells() {
-        return [];
-      },
-      dirty: true
-    } as any;
+      dirty: false,
+      getMetadata: jest.fn((key: string) => {
+        if (key === 'deepnote') {
+          return deepnoteMetadata;
+        }
+        return undefined;
+      })
+    };
 
-    panel = {
+    return {
       context: {
         ready: Promise.resolve(),
-        model: {
-          getMetadata: jest.fn().mockReturnValue({
-            notebooks: {
-              nb1: { id: 'nb1', name: 'nb1', cells: [{ source: 'code' }] },
-              nb2: { id: 'nb2', name: 'nb2', cells: [] }
-            },
-            notebook_names: ['nb1', 'nb2']
-          })
-        }
+        model: mockNotebookModel as INotebookModel
       },
-      model
-    } as any;
+      model: mockNotebookModel as INotebookModel
+    } as unknown as NotebookPanel;
+  };
 
-    // Attach to DOM
-    const widget = new NotebookPicker(panel);
-    // Override onAfterAttach to avoid errors from this.parent being null
-    (widget as any).onAfterAttach = jest.fn();
+  const attachWidget = async (panel: NotebookPanel): Promise<void> => {
+    widget = new NotebookPicker(panel);
     Widget.attach(widget, document.body);
+    // Wait for widget to attach and render
     await framePromise();
-  });
+    // Wait for constructor's async initialization to complete
+    await Promise.resolve();
+    await framePromise();
+  };
 
   afterEach(() => {
-    document.body.innerHTML = '';
-    jest.restoreAllMocks();
+    if (consoleErrorSpy) {
+      consoleErrorSpy.mockRestore();
+      consoleErrorSpy = null;
+    }
+    if (widget && !widget.isDisposed) {
+      widget.dispose();
+    }
+    // Clean up DOM
+    const attached = document.querySelectorAll('.jp-ReactWidget');
+    attached.forEach(node => {
+      node.remove();
+    });
   });
 
-  it('should render a select element', async () => {
-    await framePromise(); // wait for rendering
-    const select = document.querySelector('select') as HTMLSelectElement;
-    expect(select).not.toBeNull();
-    expect(select.options.length).toBe(2);
-    expect(select.options[0] && select.options[0].value).toBe('nb1');
+  describe('rendering', () => {
+    it('should render a select element with notebooks', async () => {
+      const metadata = {
+        notebooks: {
+          'Notebook 1': { id: 'nb1', name: 'Notebook 1', cells: [] },
+          'Notebook 2': { id: 'nb2', name: 'Notebook 2', cells: [] }
+        }
+      };
+
+      const panel = createMockPanel(metadata);
+      await attachWidget(panel);
+
+      const select = widget.node.querySelector('select');
+      expect(select).not.toBeNull();
+      expect(select?.options.length).toBe(2);
+      expect(select?.options[0]?.value).toBe('Notebook 1');
+      expect(select?.options[1]?.value).toBe('Notebook 2');
+    });
+
+    it('should render a placeholder when no notebooks are available', async () => {
+      const metadata = {
+        notebooks: {}
+      };
+
+      const panel = createMockPanel(metadata);
+      await attachWidget(panel);
+
+      const select = widget.node.querySelector('select');
+      expect(select).not.toBeNull();
+      expect(select?.options.length).toBe(1);
+      expect(select?.options[0]?.value).toBe('-');
+    });
+
+    it('should handle invalid metadata gracefully', async () => {
+      const metadata = {
+        notebooks: null
+      } as PartialJSONObject;
+
+      const panel = createMockPanel(metadata);
+      await attachWidget(panel);
+
+      const select = widget.node.querySelector('select');
+      expect(select).not.toBeNull();
+      expect(select?.options.length).toBe(1);
+      expect(select?.options[0]?.value).toBe('-');
+    });
   });
 
-  it('should call fromJSON when selecting a notebook', async () => {
-    const select = document.querySelector('select') as HTMLSelectElement;
-    simulate(select, 'change', { target: { value: 'nb2' } });
-    await framePromise();
-    expect(model.fromJSON).toHaveBeenCalledWith(
-      expect.objectContaining({
-        cells: expect.any(Array),
-        metadata: expect.objectContaining({
-          deepnote: expect.objectContaining({
-            notebooks: expect.any(Object)
-          })
+  describe('notebook selection', () => {
+    let panel: NotebookPanel;
+
+    beforeEach(async () => {
+      const metadata = {
+        notebooks: {
+          'Notebook 1': {
+            id: 'nb1',
+            name: 'Notebook 1',
+            cells: [{ cell_type: 'code', source: 'print(1)' }]
+          },
+          'Notebook 2': {
+            id: 'nb2',
+            name: 'Notebook 2',
+            cells: [{ cell_type: 'code', source: 'print(2)' }]
+          }
+        }
+      };
+
+      panel = createMockPanel(metadata);
+      await attachWidget(panel);
+    });
+
+    it('should call fromJSON when selecting a different notebook', async () => {
+      const select = widget.node.querySelector('select') as HTMLSelectElement;
+      expect(select).not.toBeNull();
+
+      select.value = 'Notebook 2';
+      select.dispatchEvent(new Event('change', { bubbles: true }));
+      await framePromise();
+
+      expect(mockNotebookModel.fromJSON).toHaveBeenCalledTimes(1);
+      expect(mockNotebookModel.fromJSON).toHaveBeenCalledWith(
+        expect.objectContaining({
+          cells: [{ cell_type: 'code', source: 'print(2)' }],
+          metadata: {
+            deepnote: {
+              notebooks: expect.objectContaining({
+                'Notebook 1': expect.any(Object),
+                'Notebook 2': expect.any(Object)
+              })
+            }
+          },
+          nbformat: 4,
+          nbformat_minor: 0
         })
-      })
-    );
-  });
+      );
+    });
 
-  it('should not call fromJSON if selected notebook is invalid', async () => {
-    const getMetadata = panel.context.model.getMetadata as jest.Mock;
-    getMetadata.mockReturnValue({ notebooks: {}, notebook_names: [] });
+    it('should set model.dirty to false after switching notebooks', async () => {
+      const select = widget.node.querySelector('select') as HTMLSelectElement;
+      select.value = 'Notebook 2';
+      select.dispatchEvent(new Event('change', { bubbles: true }));
+      await framePromise();
 
-    const select = document.querySelector('select') as HTMLSelectElement;
-    simulate(select, 'change', { target: { value: 'nonexistent' } });
-    await framePromise();
-    expect(model.fromJSON).not.toHaveBeenCalled();
+      expect(mockNotebookModel.dirty).toBe(false);
+    });
+
+    it('should not call fromJSON when selecting a non-existent notebook', async () => {
+      const select = widget.node.querySelector('select') as HTMLSelectElement;
+      select.value = 'NonExistent';
+      select.dispatchEvent(new Event('change', { bubbles: true }));
+      await framePromise();
+
+      expect(mockNotebookModel.fromJSON).not.toHaveBeenCalled();
+    });
+
+    it('should not call fromJSON when panel.model is null', async () => {
+      widget.dispose();
+
+      // Create panel with null model
+      const nullModelPanel = {
+        context: {
+          ready: Promise.resolve(),
+          model: {
+            getMetadata: jest.fn(() => deepnoteMetadata)
+          }
+        },
+        model: null
+      } as unknown as NotebookPanel;
+
+      await attachWidget(nullModelPanel);
+
+      const select = widget.node.querySelector('select') as HTMLSelectElement;
+      select.value = 'Notebook 2';
+      select.dispatchEvent(new Event('change', { bubbles: true }));
+      await framePromise();
+
+      expect(mockNotebookModel.fromJSON).not.toHaveBeenCalled();
+    });
   });
 
-  it('should update UI after selection', async () => {
-    const select = document.querySelector('select') as HTMLSelectElement;
-    select.value = 'nb2';
-    simulate(select, 'change');
-    await framePromise();
-    expect(select.value).toBe('nb2');
+  describe('initialization', () => {
+    it('should select first notebook by default when notebooks exist', async () => {
+      const metadata = {
+        notebooks: {
+          First: { id: 'nb1', name: 'First', cells: [] },
+          Second: { id: 'nb2', name: 'Second', cells: [] }
+        }
+      };
+
+      const panel = createMockPanel(metadata);
+      await attachWidget(panel);
+
+      const select = widget.node.querySelector('select') as HTMLSelectElement;
+      expect(select.value).toBe('First');
+    });
+
+    it('should handle initialization errors gracefully', async () => {
+      consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation();
+
+      const failingPanel = {
+        context: {
+          ready: Promise.reject(new Error('Initialization failed')),
+          model: {
+            getMetadata: jest.fn(() => ({}))
+          }
+        },
+        model: mockNotebookModel as INotebookModel
+      } as unknown as NotebookPanel;
+
+      await attachWidget(failingPanel);
+      await framePromise();
+
+      expect(consoleErrorSpy).toHaveBeenCalledWith(
+        'Failed to initialize NotebookPicker:',
+        expect.any(Error)
+      );
+    });
   });
 
-  it('should handle empty metadata gracefully', async () => {
-    const getMetadata = panel.context.model.getMetadata as jest.Mock;
-    getMetadata.mockReturnValue({ notebooks: {}, notebook_names: [] });
+  describe('metadata validation', () => {
+    it('should handle invalid metadata when changing notebooks', async () => {
+      consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation();
 
-    document.body.innerHTML = '';
-    const widget = new NotebookPicker(panel);
-    // Override onAfterAttach to avoid errors from this.parent being null
-    (widget as any).onAfterAttach = jest.fn();
-    Widget.attach(widget, document.body);
-    await framePromise();
+      const metadata = {
+        notebooks: {
+          'Notebook 1': { id: 'nb1', name: 'Notebook 1', cells: [] }
+        }
+      };
+
+      const panel = createMockPanel(metadata);
+      await attachWidget(panel);
+
+      // Change the metadata to invalid format
+      deepnoteMetadata = { invalid: 'metadata' } as PartialJSONObject;
+
+      const select = widget.node.querySelector('select') as HTMLSelectElement;
+      select.value = 'Notebook 1';
+      select.dispatchEvent(new Event('change', { bubbles: true }));
+      await framePromise();
 
-    const select = document.querySelector('select') as HTMLSelectElement;
-    expect(select.options.length).toBeGreaterThanOrEqual(1);
-    expect(select.options[0] && select.options[0].value).toBe('-');
+      expect(consoleErrorSpy).toHaveBeenCalled();
+      expect(consoleErrorSpy.mock.calls[0]?.[0]).toMatch(/invalid.*metadata/i);
+      expect(mockNotebookModel.fromJSON).not.toHaveBeenCalled();
+    });
   });
 });