diff --git a/jest.config.js b/jest.config.js index db456ae..dba71c0 100644 --- a/jest.config.js +++ b/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); diff --git a/package.json b/package.json index ba570e5..8c1790e 100644 --- a/package.json +++ b/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", diff --git a/src/__tests__/NotebookPicker.spec.ts b/src/__tests__/NotebookPicker.spec.ts index 4708c5b..fde489e 100644 --- a/src/__tests__/NotebookPicker.spec.ts +++ b/src/__tests__/NotebookPicker.spec.ts @@ -1,111 +1,292 @@ // 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; + let deepnoteMetadata: PartialJSONObject; + let consoleErrorSpy: jest.SpyInstance | null = null; - beforeEach(async () => { - // Mock model + metadata - model = { + const createMockPanel = ( + metadata: PartialJSONObject, + dirty = false + ): NotebookPanel => { + deepnoteMetadata = metadata; + + mockNotebookModel = { fromJSON: jest.fn(), - get cells() { - return []; - }, - dirty: true - } as any; + dirty: dirty, + 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 => { + 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 () => { + widget.dispose(); - const select = document.querySelector('select') as HTMLSelectElement; - simulate(select, 'change', { target: { value: 'nonexistent' } }); - await framePromise(); - expect(model.fromJSON).not.toHaveBeenCalled(); + 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)' }] + } + } + }; + + const dirtyPanel = createMockPanel(metadata, true); + await attachWidget(dirtyPanel); + + const select = widget.node.querySelector('select') as HTMLSelectElement; + select.value = 'Notebook 2'; + select.dispatchEvent(new Event('change', { bubbles: true })); + await framePromise(); + + 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(); + }); }); }); diff --git a/src/components/NotebookPicker.tsx b/src/components/NotebookPicker.tsx index 1dbb941..102f7e3 100644 --- a/src/components/NotebookPicker.tsx +++ b/src/components/NotebookPicker.tsx @@ -12,18 +12,23 @@ export class NotebookPicker extends ReactWidget { constructor(private panel: NotebookPanel) { super(); - void panel.context.ready.then(() => { - const deepnoteMetadata = this.panel.context.model.getMetadata('deepnote'); - const metadataNames = deepnoteMetadata?.notebook_names; - const names = - Array.isArray(metadataNames) && - metadataNames.every(n => typeof n === 'string') - ? metadataNames - : []; + panel.context.ready + .then(() => { + const deepnoteMetadata = + this.panel.context.model.getMetadata('deepnote'); + const metadataNames = deepnoteMetadata?.notebook_names; + const names = + Array.isArray(metadataNames) && + metadataNames.every(n => typeof n === 'string') + ? metadataNames + : []; - this.selected = names.length === 0 ? null : (names[0] ?? null); - this.update(); - }); + this.selected = names.length === 0 ? null : (names[0] ?? null); + this.update(); + }) + .catch(error => { + console.error('Failed to initialize NotebookPicker:', error); + }); } private handleChange = (event: React.ChangeEvent) => { @@ -68,7 +73,9 @@ export class NotebookPicker extends ReactWidget { protected onAfterAttach(msg: Message): void { super.onAfterAttach(msg); requestAnimationFrame(() => { - MessageLoop.sendMessage(this.parent!, Widget.ResizeMessage.UnknownSize); + if (this.parent) { + MessageLoop.sendMessage(this.parent, Widget.ResizeMessage.UnknownSize); + } }); } diff --git a/src/handler.ts b/src/handler.ts index d4c619b..4e2bf1d 100644 --- a/src/handler.ts +++ b/src/handler.ts @@ -25,21 +25,27 @@ export async function requestAPI( try { response = await ServerConnection.makeRequest(requestUrl, init, settings); } catch (error) { - throw new ServerConnection.NetworkError(error as any); + throw new ServerConnection.NetworkError( + error instanceof TypeError ? error : new TypeError(String(error)) + ); } - let data: any = await response.text(); + let data: string | unknown = await response.text(); - if (data.length > 0) { + if (typeof data === 'string' && data.length > 0) { try { data = JSON.parse(data); - } catch (error) { + } catch { console.log('Not a JSON response body.', response); } } if (!response.ok) { - throw new ServerConnection.ResponseError(response, data.message || data); + const errorMessage = + data && typeof data === 'object' && 'message' in data + ? (data as { message: string }).message + : String(data); + throw new ServerConnection.ResponseError(response, errorMessage); } return data;