remove undici

[juliangruber]

This is a semver major change, because it drops Node 16 support

[socket-security]

New and updated dependencies detected. Learn more about Socket for GitHub ↗︎

Packages	Version	New capabilities	Transitives	Size	Publisher
@types/node	20.8.0	None	+0	3.92 MB	types
undici	5.25.2...5.25.3	environment	+1/-2	1.22 MB	matteo.collina

[juliangruber]

I feel like I'm doing something wrong here. However, MDN suggests this approach. I didn't find anything in node's stdlib for this 🤔

[juliangruber]

is there a better lib target to add? See https://www.typescriptlang.org/tsconfig#lib. DOM was suggested in stack overflow posts

[bajtos]

Oh man, this is a rabbit hole. See the discussion in DefinitelyTyped/DefinitelyTyped#60924.

First of all, Station Core is currently supporting Node.js 16.x which does not provide a built-in Fetch API. So the first step is to upgrade our minimal supported version to 18.x - I think this is a breaking change.

Then, ideally, all we need to do is add @types/node@18 to our dev-dependencies.

However, that does not work because of the reasons described in DefinitelyTyped/DefinitelyTyped#60924.

Also, we should not be using DOM types, because Fetch API on server has slightly different API than in the browser. (This was mentioned in that DefinitelyTyped discussion too.)

I quite like the workaround described DefinitelyTyped/DefinitelyTyped#60924 (comment). The downside is that we still need to have undici in our dependencies (and probably keep it up to date via Dependabot?), but at least it becomes a dev-time only dependency. 🤷🏻‍♂️

// fetch.d.ts
import type * as undici from "undici";

declare global {
  // Re-export undici fetch function and various classes to global scope.
  // These are classes and functions expected to be at global scope according to
  // Node.js v18 API documentation.
  // See: https://nodejs.org/dist/latest-v18.x/docs/api/globals.html
  export const { 
    FormData,
    Headers,
    Request,
    Response,
    fetch,
  }: typeof undici;
}

[bajtos]

Here is a diff of the changes I think we should make on our way to fix this properly. Unfortunately, it's not enough.

diff --git a/package.json b/package.json
index edc5522..abe92f3 100644
--- a/package.json
+++ b/package.json
@@ -11,7 +11,7 @@
     "station": "./bin/station.js"
   },
   "engines": {
-    "node": ">=16"
+    "node": ">=18"
   },
   "scripts": {
     "build": "tsc -p .",
@@ -32,6 +32,7 @@
     ]
   },
   "devDependencies": {
+    "@types/node": "^18.18.1",
     "@types/yargs": "^17.0.24",
     "get-stream": "^8.0.1",
     "mocha": "^10.2.0",
diff --git a/tsconfig.json b/tsconfig.json
index 5d96c50..83db0fb 100644
--- a/tsconfig.json
+++ b/tsconfig.json
@@ -3,7 +3,7 @@
     "allowJs": true,
     "checkJs": true,
     "skipLibCheck": true,
-    "lib": ["es2021", "DOM"],
+    "lib": ["es2021"],
     "target": "es2021",
     "module": "Node16",
     "moduleResolution": "node16",

[juliangruber]

Thank you for doing the research! I've applied the fixes you suggested. I don't think overall this improves our situation unfortunately, and I'd rather propose to continue using undici for now

[bajtos]

I don't think overall this improves our situation unfortunately, and I'd rather propose to continue using undici for now

💯 agree

[bajtos]

FWIW, this should be ^18.x.y (for the latest x and y). Otherwise, the TypeScript compiler will allow us to use APIs introduced in Node.js 20 that are not available in Node.js 18 yet.