huw
commented
11 months ago Diagnosis
It appears that this ends up running into some fundamental TypeScript issues. I am not an expert here, but what I understand is happening is that TypeScript imports your tsconfig.json types field first, then overrides it with anything imported from a dependency. If you have even a single dependency that overrides a global type that you’re using from @cloudflare/workers-types, then too bad, that type is overridden.
This is a big and hard to solve problem, because TypeScript automatically emits /// <reference types="node" /> any time a dependency uses a Node global. My pnpm why @types/node is showing tens of projects with these globals; it’s infeasible for me to personally patch or PR each of these dependencies to fix their projects, even if you buy that those projects have made some mistake in the first place.
Unfortunately, TypeScript can’t fix this problem without a much bigger rearchitecture (microsoft/TypeScript#18588; the strict-env proposal has also stalled). If there’s any silver lining to this, I guess it’s that it would also break other WinterCG projects that use TypeScript like this (IIRC the only one would be @edge-runtime/types, and only if they have a similar type incompatibility; Deno and others do their global type checking differently or just use @types/node).
Aside: A better, cheap workaround
A slightly better workaround than the above would be to add the following to a global.d.ts in your project:
declare module "@types/node" {
declare const value: unknown;
export default value;
}This will limit the damage of neutralising @types/node to just the projects you’re using Workers in, and allow you to keep @types/node unpinned for actual node projects (such as scripts or other packages in a monorepo).
The long-term, idiomatic solutions
TypeScript 5.0 includes a new compilerOptions directive, customConditions. This means the compiler can now resolve package.json exports conditions to locate types depending on the runtime/conditions that are present. In short, this means that packages which emit a /// <reference types="node" /> can scope this to just be imported by the node condition, or to remove it for the workerd condition. This is the TypeScript equivalent of a bundler’s conditions directive (ex. ESBuild), and you use it in much the same way.
However, as we’ve found with the workerd condition in the ecosystem, this requires every package to update to split their exports. I’m not convinced this is feasible. An easier solution would be for @types/node to add a workerd condition with a compatible export, like so:
"exports": {
"workerd": {
"node": {
"types": "./index.workerd.d.ts",
},
"types": "./noop.d.ts"
},
"types": "./index.d.ts"
}By default, this would just be a no-op, which trusts that you’re importing @cloudflare/workers-types and neutralises @types/node. If you’re running Workers by default, this would most accurately reflect the types you’d have access to. But the beauty of this solution is that if you’re running in Node compatibility mode, you could specify "customConditions": ["workerd", "node"] and we’d serve up a version with all of the incompatible types removed. This would most accurately reflect Node compat mode, where we want a lot of the proper node types, but Cloudflare-augmented globals would still be present. We could even omit anything that’s not supported in Node compat mode—but I assume Cloudflare would be responsible for maintaining this file.
I’ll hit up the DefinitelyTyped team and see what they think—an opinion from Cloudflare would be greatly appreciated here also :)
TL;DR just give me a patch
First, add this to your tsconfig.json:
{
"compilerOptions": {
"customConditions": ["workerd", "worker", "browser"],
}
}Second, patch @types/node/package.json using your package manager to add:
"exports": {
"workerd": {
"types": "./noop.d.ts"
},
"types": "./index.d.ts"
}And add @types/node/noop.d.ts to your patch—it just needs to be empty. Here’s a patchfile for pnpm:
diff --git a/index.workerd.d.ts b/index.workerd.d.ts
new file mode 100644
index 0000000000000000000000000000000000000000..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
diff --git a/package.json b/package.json
index e9e15c90021711122a01a324f07f3675e13e1e14..b04e27142c8ef196d8bb3bb53661794fde3569e7 100644
--- a/package.json
+++ b/package.json
@@ -213,6 +213,12 @@
],
"main": "",
"types": "index.d.ts",
+ "exports": {
+ "workerd": {
+ "types": "./index.workerd.d.ts"
+ },
+ "types": "./index.d.ts"
+ },
"typesVersions": {
"<=4.8": {
"*": [
Cherry
Skye-31
chitalian
smcgivern
punkeel
irvinebroque
petebacondarwin
Lp-Francois
jakebailey
chalkygames123
As per https://github.com/DefinitelyTyped/DefinitelyTyped/pull/66824 which was merged recently and released in
@types/node@20.8.4, Node.js now defines its ownRequest,Response, etc. types viaundici-types.While this is great in Node environments, it completely messes with the
RequestandResponse(maybe others?) types defined by@cloudflare/workers-types.Example code that used to work before:
Now has a type error with:
Expected 0 type arguments, but got 1..Or:
Now has a type error with `Property 'cf' does not exist on type 'Request'.
Or:
Now has a type error with:
Why load
@types/node?This is a fair question, and you generally don't need to load
@types/nodeat all, and in fact, I don't directly. But, many dependencies that people use, do in fact require@types/node, including the likes of@types/pg, which is loaded by@neondatabase/serverless.It seems that once this newer version of
@types/nodeis loaded into memory at all, it just overrides the globally defined Request/Response/fetch types from@cloudflare/workers-types.Full reproduction
See the following github repo: https://github.com/Cherry/workers-node-types-issue-repro.
This showcases an example package that loads
@types/node, and if you comment in/out the import you'll see the type errors now show up.Workaround
My current workaround for this is to pin
@types/nodevia npm overrides, with this in mypackage.json:It's a very naive and short-term workaround, but works for now.