Remove node-fetch dependency (#15969)

* Remove `node-fetch` * update to node 20 * use node 20 on CI * Add custom hest-env * remove custom node_version override * Align with empty status text * more fixes * fix Response instantiation for 204 * fix expect * fix handling of 205 * streamline expects * Update to Node 20 in the contributing guide
jupyterlab · Mar 14, 2024 · 412cb0a · 412cb0a
1 parent 783880f
commit 412cb0a
Show file tree

Hide file tree

Showing 18 changed files with 64 additions and 78 deletions.
diff --git a/.gitpod.yml b/.gitpod.yml
@@ -25,7 +25,7 @@ tasks:
       EOT
       source /workspace/bin/activate-env.sh
       micromamba config append channels conda-forge
-      micromamba install -n base -y python=3.10 nodejs=18 yarn
+      micromamba install -n base -y python=3.10 nodejs=20 yarn
       source /workspace/bin/activate-env.sh
       python -m pip install -e ".[dev,docs,test]" && jlpm install && jlpm run build
       gp sync-done setup

diff --git a/buildutils/src/ensure-repo.ts b/buildutils/src/ensure-repo.ts
@@ -120,7 +120,6 @@ const UNUSED: Dict<string[]> = {
     '@babel/core',
     '@babel/preset-env',
     'fs-extra',
-    'node-fetch',
     'identity-obj-proxy',
     'jest-environment-jsdom',
     'jest-junit'

diff --git a/docs/source/developer/contributing.rst b/docs/source/developer/contributing.rst
@@ -309,7 +309,7 @@ Installing Node.js and jlpm
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Building JupyterLab from its GitHub source code requires Node.js. The
-development version requires Node.js version 18+, as defined in the
+development version requires Node.js version 20+, as defined in the
 ``engines`` specification in
 `dev_mode/package.json <https://github.com/jupyterlab/jupyterlab/blob/main/dev_mode/package.json>`__.
 

diff --git a/docs/source/extension/extension_tutorial.rst b/docs/source/extension/extension_tutorial.rst
@@ -66,7 +66,7 @@ new environment named ``jupyterlab-ext``.
 
 .. code:: bash
 
-    conda create -n jupyterlab-ext --override-channels --strict-channel-priority -c conda-forge -c nodefaults jupyterlab=4 nodejs=18 git copier=7 jinja2-time
+    conda create -n jupyterlab-ext --override-channels --strict-channel-priority -c conda-forge -c nodefaults jupyterlab=4 nodejs=20 git copier=7 jinja2-time
 
 Now activate the new environment so that all further commands you run
 work out of that environment.

diff --git a/packages/docregistry/test/context.spec.ts b/packages/docregistry/test/context.spec.ts
@@ -259,7 +259,7 @@ describe('docregistry/context', () => {
         });
 
         await expect(context.initialize(true)).rejects.toThrow(
-          'Invalid response: 403 Forbidden'
+          /Invalid response: 403/
         );
         expect(called).toBe(2);
         expect(checked).toBe('failed');

diff --git a/packages/services/test/config/config.spec.ts b/packages/services/test/config/config.spec.ts
@@ -50,9 +50,7 @@ describe('config', () => {
         name: randomName(),
         serverSettings
       });
-      await expect(configPromise).rejects.toThrow(
-        /Invalid response: 201 Created/
-      );
+      await expect(configPromise).rejects.toThrow(/Invalid response: 201/);
     });
   });
 
@@ -83,7 +81,7 @@ describe('config', () => {
       const config = await ConfigSection.create({ name: randomName() });
       handleRequest(config, 201, {});
       const update = config.update({ foo: 'baz' });
-      await expect(update).rejects.toThrow(/Invalid response: 201 Created/);
+      await expect(update).rejects.toThrow(/Invalid response: 201/);
     });
   });
 });
@@ -198,7 +196,7 @@ describe('jupyter.services - ConfigWithDefaults', () => {
       const config = new ConfigWithDefaults({ section });
       const set = config.set('foo', 'bar');
       expect(section.data['foo']).toBe('bar');
-      await expectFailure(set, 'Invalid response: 201 Created');
+      await expectFailure(set, 'Invalid response: 201');
     });
   });
 });
diff --git a/packages/services/test/contents/index.spec.ts b/packages/services/test/contents/index.spec.ts
@@ -260,7 +260,7 @@ describe('contents', () => {
     it('should fail for an incorrect response', async () => {
       handleRequest(contents, 201, DEFAULT_DIR);
       const get = contents.get('/foo');
-      await expect(get).rejects.toThrow(/Invalid response: 201 Created/);
+      await expect(get).rejects.toThrow(/Invalid response: 201/);
     });
 
     it('should store original server path for directory', async () => {
@@ -393,7 +393,7 @@ describe('contents', () => {
     it('should fail for an incorrect response', async () => {
       handleRequest(contents, 200, DEFAULT_DIR);
       const newDir = contents.newUntitled();
-      await expect(newDir).rejects.toThrow(/Invalid response: 200 OK/);
+      await expect(newDir).rejects.toThrow(/Invalid response: 200/);
     });
   });
 
@@ -428,7 +428,7 @@ describe('contents', () => {
     it('should fail for an incorrect response', async () => {
       handleRequest(contents, 200, {});
       const del = contents.delete('/foo/bar.txt');
-      await expect(del).rejects.toThrow(/Invalid response: 200 OK/);
+      await expect(del).rejects.toThrow(/Invalid response: 200/);
     });
 
     it('should throw a specific error', async () => {
@@ -488,7 +488,7 @@ describe('contents', () => {
     it('should fail for an incorrect response', async () => {
       handleRequest(contents, 201, DEFAULT_FILE);
       const rename = contents.rename('/foo/bar.txt', '/foo/baz.txt');
-      await expect(rename).rejects.toThrow(/Invalid response: 201 Created/);
+      await expect(rename).rejects.toThrow(/Invalid response: 201/);
     });
   });
 
@@ -540,7 +540,7 @@ describe('contents', () => {
     it('should fail for an incorrect response', async () => {
       handleRequest(contents, 204, DEFAULT_FILE);
       const save = contents.save('/foo', { type: 'file', name: 'test' });
-      await expect(save).rejects.toThrow(/Invalid response: 204 No Content/);
+      await expect(save).rejects.toThrow(/Invalid response: 204/);
     });
   });
 
@@ -583,7 +583,7 @@ describe('contents', () => {
     it('should fail for an incorrect response', async () => {
       handleRequest(contents, 200, DEFAULT_FILE);
       const copy = contents.copy('/foo/bar.txt', '/baz');
-      await expect(copy).rejects.toThrow(/Invalid response: 200 OK/);
+      await expect(copy).rejects.toThrow(/Invalid response: 200/);
     });
   });
 
@@ -615,7 +615,7 @@ describe('contents', () => {
     it('should fail for an incorrect response', async () => {
       handleRequest(contents, 200, DEFAULT_CP);
       const checkpoint = contents.createCheckpoint('/foo/bar.txt');
-      await expect(checkpoint).rejects.toThrow(/Invalid response: 200 OK/);
+      await expect(checkpoint).rejects.toThrow(/Invalid response: 200/);
     });
   });
 
@@ -650,9 +650,7 @@ describe('contents', () => {
     it('should fail for an incorrect response', async () => {
       handleRequest(contents, 201, {});
       const checkpoints = contents.listCheckpoints('/foo/bar.txt');
-      await expect(checkpoints).rejects.toThrow(
-        /Invalid response: 201 Created/
-      );
+      await expect(checkpoints).rejects.toThrow(/Invalid response: 201/);
     });
   });
 
@@ -683,7 +681,7 @@ describe('contents', () => {
         '/foo/bar.txt',
         DEFAULT_CP.id
       );
-      await expect(checkpoint).rejects.toThrow(/Invalid response: 200 OK/);
+      await expect(checkpoint).rejects.toThrow(/Invalid response: 200/);
     });
   });
 
@@ -710,7 +708,7 @@ describe('contents', () => {
         '/foo/bar.txt',
         DEFAULT_CP.id
       );
-      await expect(checkpoint).rejects.toThrow(/Invalid response: 200 OK/);
+      await expect(checkpoint).rejects.toThrow(/Invalid response: 200/);
     });
   });
 });
@@ -821,7 +819,7 @@ describe('drive', () => {
       const drive = new Drive();
       handleRequest(drive, 201, DEFAULT_DIR);
       const get = drive.get('/foo');
-      await expect(get).rejects.toThrow(/Invalid response: 201 Created/);
+      await expect(get).rejects.toThrow(/Invalid response: 201/);
     });
   });
 
@@ -918,7 +916,7 @@ describe('drive', () => {
       const drive = new Drive();
       handleRequest(drive, 200, DEFAULT_DIR);
       const newDir = drive.newUntitled();
-      await expect(newDir).rejects.toThrow(/Invalid response: 200 OK/);
+      await expect(newDir).rejects.toThrow(/Invalid response: 200/);
     });
   });
 
@@ -953,7 +951,7 @@ describe('drive', () => {
       const drive = new Drive();
       handleRequest(drive, 200, {});
       const del = drive.delete('/foo/bar.txt');
-      await expect(del).rejects.toThrow(/Invalid response: 200 OK/);
+      await expect(del).rejects.toThrow(/Invalid response: 200/);
     });
 
     it('should throw a specific error', async () => {
@@ -1015,7 +1013,7 @@ describe('drive', () => {
       const drive = new Drive();
       handleRequest(drive, 201, DEFAULT_FILE);
       const rename = drive.rename('/foo/bar.txt', '/foo/baz.txt');
-      await expect(rename).rejects.toThrow(/Invalid response: 201 Created/);
+      await expect(rename).rejects.toThrow(/Invalid response: 201/);
     });
   });
 
@@ -1071,7 +1069,7 @@ describe('drive', () => {
       const drive = new Drive();
       handleRequest(drive, 204, DEFAULT_FILE);
       const save = drive.save('/foo', { type: 'file', name: 'test' });
-      await expect(save).rejects.toThrow(/Invalid response: 204 No Content/);
+      await expect(save).rejects.toThrow(/Invalid response: 204/);
     });
   });
 
@@ -1117,7 +1115,7 @@ describe('drive', () => {
       const drive = new Drive();
       handleRequest(drive, 200, DEFAULT_FILE);
       const copy = drive.copy('/foo/bar.txt', '/baz');
-      await expect(copy).rejects.toThrow(/Invalid response: 200 OK/);
+      await expect(copy).rejects.toThrow(/Invalid response: 200/);
     });
   });
 
@@ -1151,7 +1149,7 @@ describe('drive', () => {
       const drive = new Drive();
       handleRequest(drive, 200, DEFAULT_CP);
       const checkpoint = drive.createCheckpoint('/foo/bar.txt');
-      await expect(checkpoint).rejects.toThrow(/Invalid response: 200 OK/);
+      await expect(checkpoint).rejects.toThrow(/Invalid response: 200/);
     });
   });
 
@@ -1188,9 +1186,7 @@ describe('drive', () => {
       const drive = new Drive();
       handleRequest(drive, 201, {});
       const checkpoints = drive.listCheckpoints('/foo/bar.txt');
-      await expect(checkpoints).rejects.toThrow(
-        /Invalid response: 201 Created/
-      );
+      await expect(checkpoints).rejects.toThrow(/Invalid response: 201/);
     });
   });
 
@@ -1213,7 +1209,7 @@ describe('drive', () => {
       const drive = new Drive();
       handleRequest(drive, 200, {});
       const checkpoint = drive.restoreCheckpoint('/foo/bar.txt', DEFAULT_CP.id);
-      await expect(checkpoint).rejects.toThrow(/Invalid response: 200 OK/);
+      await expect(checkpoint).rejects.toThrow(/Invalid response: 200/);
     });
   });
 
@@ -1238,7 +1234,7 @@ describe('drive', () => {
       const drive = new Drive();
       handleRequest(drive, 200, {});
       const checkpoint = drive.deleteCheckpoint('/foo/bar.txt', DEFAULT_CP.id);
-      await expect(checkpoint).rejects.toThrow(/Invalid response: 200 OK/);
+      await expect(checkpoint).rejects.toThrow(/Invalid response: 200/);
     });
   });
 

diff --git a/packages/services/test/kernel/ikernel.spec.ts b/packages/services/test/kernel/ikernel.spec.ts
@@ -620,7 +620,7 @@ describe('Kernel.IKernel', () => {
         name: defaultKernel.name
       });
       const interrupt = defaultKernel.interrupt();
-      await expect(interrupt).rejects.toThrow(/Invalid response: 200 OK/);
+      await expect(interrupt).rejects.toThrow(/Invalid response: 200/);
     });
 
     it('should throw an error for an error response', async () => {
@@ -668,7 +668,7 @@ describe('Kernel.IKernel', () => {
       const { id, name } = defaultKernel;
       handleRequest(defaultKernel, 205, { id, name });
       await expect(defaultKernel.restart()).rejects.toThrow(
-        /Invalid response: 205 Reset Content/
+        /Invalid response: 205/
       );
     });
 
@@ -737,7 +737,7 @@ describe('Kernel.IKernel', () => {
         name: 'foo'
       });
       const shutdown = defaultKernel.shutdown();
-      await expect(shutdown).rejects.toThrow(/Invalid response: 200 OK/);
+      await expect(shutdown).rejects.toThrow(/Invalid response: 200/);
     });
 
     it('should handle a 404 error', async () => {

diff --git a/packages/services/test/kernel/kernel.spec.ts b/packages/services/test/kernel/kernel.spec.ts
@@ -65,7 +65,7 @@ describe('kernel', () => {
     it('should throw an error for an invalid response', async () => {
       const settings = getRequestHandler(201, {});
       const promise = KernelAPI.listRunning(settings);
-      await expect(promise).rejects.toThrow(/Invalid response: 201 Created/);
+      await expect(promise).rejects.toThrow(/Invalid response: 201/);
     });
 
     it('should throw an error for an error response', async () => {
@@ -116,7 +116,7 @@ describe('kernel', () => {
       const data = { id: UUID.uuid4(), name: 'foo' };
       const serverSettings = getRequestHandler(200, data);
       const kernelPromise = KernelAPI.startNew({}, serverSettings);
-      await expect(kernelPromise).rejects.toThrow(/Invalid response: 200 OK/);
+      await expect(kernelPromise).rejects.toThrow(/Invalid response: 200/);
     });
 
     it('should throw an error for an error response', async () => {

diff --git a/packages/services/test/kernelspec/kernelspec.spec.ts b/packages/services/test/kernelspec/kernelspec.spec.ts
@@ -121,7 +121,7 @@ describe('kernel', () => {
     it('should throw an error for an invalid response', async () => {
       const serverSettings = getRequestHandler(201, {});
       const promise = KernelSpecAPI.getSpecs(serverSettings);
-      await expect(promise).rejects.toThrow(/Invalid response: 201 Created/);
+      await expect(promise).rejects.toThrow(/Invalid response: 201/);
     });
 
     it('should handle metadata', async () => {

diff --git a/packages/services/test/serverconnection.spec.ts b/packages/services/test/serverconnection.spec.ts
@@ -26,7 +26,7 @@ describe('ServerConnection', () => {
         {},
         settings
       );
-      expect(response.statusText).toBe('OK');
+      expect(response.status).toBe(200);
       const data = await response.json();
       expect(data).toBe('hello');
     });
@@ -87,7 +87,7 @@ describe('ServerConnection', () => {
         settings
       );
       const err = new ServerConnection.ResponseError(response);
-      expect(err.message).toBe('Invalid response: 200 OK');
+      expect(err.message).toBe('Invalid response: 200 ');
     });
   });
 

diff --git a/packages/services/test/utils.ts b/packages/services/test/utils.ts
@@ -100,15 +100,15 @@ export function getRequestHandler(
   // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
   body: any
 ): ServerConnection.ISettings {
-  const fetch = (info: RequestInfo, init: RequestInit) => {
+  const customFetch = (info: RequestInfo, init?: RequestInit) => {
     // Normalize the body.
     body = JSON.stringify(body);
 
     // Create the response and return it as a promise.
     const response = new Response(body, { status });
     return Promise.resolve(response as any);
   };
-  return ServerConnection.makeSettings({ fetch });
+  return ServerConnection.makeSettings({ fetch: customFetch });
 }
 
 /**
@@ -135,6 +135,10 @@ export function handleRequest(item: IService, status: number, body: any): void {
     if (typeof body !== 'string') {
       body = JSON.stringify(body);
     }
+    // Body should be null for these status codes
+    if (status === 204 || status === 205) {
+      body = null;
+    }
 
     // Create the response and return it as a promise.
     const response = new Response(body, { status });

diff --git a/packages/testing/package.json b/packages/testing/package.json
@@ -44,14 +44,12 @@
     "jest": "^29.2.0",
     "jest-environment-jsdom": "^29.3.0",
     "jest-junit": "^15.0.0",
-    "node-fetch": "^2.6.0",
     "simulate-event": "~1.4.0",
     "ts-jest": "^29.1.0"
   },
   "devDependencies": {
     "@types/jest": "^29.2.0",
     "@types/node": "^18.11.18",
-    "@types/node-fetch": "^2.6.2",
     "rimraf": "~5.0.5",
     "typescript": "~5.1.6"
   },

diff --git a/packages/testing/src/jest-config.ts b/packages/testing/src/jest-config.ts
@@ -22,7 +22,7 @@ const esModules = [
 
 module.exports = function (baseDir: string) {
   return {
-    testEnvironment: 'jsdom',
+    testEnvironment: '@jupyterlab/testing/lib/jest-env.js',
     moduleNameMapper: {
       '\\.(css|less|sass|scss)$': 'identity-obj-proxy',
       '\\.(gif|ttf|eot)$': '@jupyterlab/testing/lib/jest-file-mock.js'