workerd: Transitive loading of `@types/node` breaks Request/Response (etc.) types
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 own Request, Response, etc. types via undici-types.
While this is great in Node environments, it completely messes with the Request and Response (maybe others?) types defined by @cloudflare/workers-types.
Example code that used to work before:
type SomeType = {
foo: string
};
const response = await fetch(...);
const json = await response.json<SomeType>()
Now has a type error with: Expected 0 type arguments, but got 1..
Or:
const country = request?.cf?.country;
Now has a type error with `Property ‘cf’ does not exist on type ‘Request’.
Or:
new Request(..., request);
Now has a type error with:
Argument of type 'Request' is not assignable to parameter of type 'RequestInit'.
Types of property 'referrerPolicy' are incompatible.
Type 'string' is not assignable to type 'ReferrerPolicy | undefined'.
Why load @types/node?
This is a fair question, and you generally don’t need to load @types/node at 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/node is 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/node via npm overrides, with this in my package.json:
{
"overrides": {
"@types/node": "20.8.3"
}
}
It’s a very naive and short-term workaround, but works for now.
About this issue
- Original URL
- State: open
- Created 9 months ago
- Reactions: 31
- Comments: 17 (3 by maintainers)
Commits related to this issue
- Rebuild pnpm-lock.yaml and override all versions of @types/node The override is necessary due to https://github.com/cloudflare/workerd/issues/1298 — committed to AdiRishi/turborepo-remote-cache-cloudflare by AdiRishi 8 months ago
- refactor: Fix type errors with @types/node See also https://github.com/cloudflare/workerd/issues/1298 for more information — committed to serlo/cloudflare-worker by kulla 7 months ago
- fix(tests): remove @node/types (#386) stampy.ts tests were failing (e.g. https://github.com/StampyAI/stampy-ui/actions/runs/7854687158/job/21435971107) with the following error: ``` app/serve... — committed to StampyAI/stampy-ui by jrhender 5 months ago
- chore: fix type issues found after upgrading dependencies, see: https://github.com/cloudflare/workerd/issues/1298 — committed to cloudflare/serverless-registry by gabivlj 4 months ago
- chore: fix type issues found after upgrading dependencies, see: https://github.com/cloudflare/workerd/issues/1298 — committed to cloudflare/serverless-registry by gabivlj 4 months ago
- chore: fix type issues found after upgrading dependencies, see: https://github.com/cloudflare/workerd/issues/1298 — committed to cloudflare/serverless-registry by gabivlj 4 months ago
- chore: fix type issues found after upgrading dependencies, see: https://github.com/cloudflare/workerd/issues/1298 — committed to cloudflare/serverless-registry by gabivlj 4 months ago
- chore: fix type issues found after upgrading dependencies, see: https://github.com/cloudflare/workerd/issues/1298 — committed to harryzcy/serverless-registry by gabivlj 4 months ago
- fix: Add Github Actions and fix type errors(#18) History of this squashed commit: * Fix a test and type error * Add github action tests * Pass test in both node 18 and 20 * Remove some unus... — committed to cloudflare/serverless-registry by harryzcy 4 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.jsontypesfield 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. Mypnpm why @types/nodeis 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-envproposal 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.tsin your project:This will limit the damage of neutralising
@types/nodeto just the projects you’re using Workers in, and allow you to keep@types/nodeunpinned for actual node projects (such as scripts or other packages in a monorepo).The long-term, idiomatic solutions
TypeScript 5.0 includes a new
compilerOptionsdirective,customConditions. This means the compiler can now resolve package.jsonexportsconditions 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 thenodecondition, or to remove it for theworkerdcondition. This is the TypeScript equivalent of a bundler’sconditionsdirective (ex. ESBuild), and you use it in much the same way.However, as we’ve found with the
workerdcondition 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/nodeto add aworkerdcondition with a compatible export, like so:By default, this would just be a no-op, which trusts that you’re importing
@cloudflare/workers-typesand 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:Second, patch
@types/node/package.jsonusing your package manager to add:And add
@types/node/noop.d.tsto your patch—it just needs to be empty. Here’s a patchfile for pnpm:For anyone else running into this issue here is what I am trying to solve…
Problem - type conflicts
We keep getting type conflicts within our edittors and when trying to build our project.
The issue is because there are two version of Response/Request and fetch that were added to node types 20^.
Solutions
I tried to use @huw’s solution here however, I was still getting a ton of different compile and type checking issues.
using pnpm to override the node types version to 18 works for now but this is not a long term solution
or for yarn…
Thank you all for your help!
Now that
@types/nodev18 has been updated to include fetch types, I suspect this issue will show up a lot more frequently. So it’s worth also adding that there are a few ways to handle having multiple versions of@types/nodein your project that need patching:pnpm -r upgrade @types/node(or equivalent) to upgrade to the latest satisfiable versions. In my monorepo this got me down to a patch for v18 and v20@types/nodeto the version you’re using and just patch it onceI have gone with (3), which looks like (
package.json):For me, this means updating the patch whenever
@types/nodeupdates, but usually that just involves renaming the file cause theirpackage.jsonisn’t going to change a tonne.Thanks! No worries, I appreciate your time @huw thanks
This link should work, but just so we’re clear I don’t have the time to help you there (I don’t work for Cloudflare just FYI), I’m just redirecting you to the right place to ask for it 😃
Reading Andrew’s comments, it looks like the way forward is for Cloudflare (or the community) to maintain their own fork of
@types/nodewith the above changes, and encouraging users to override their dependencies. I think it would be enough for that package to just/// <reference types="node" />forindex.node.d.ts, and have a blank file atindex.workerd.d.ts, in which case it could just have a peer dependency on@types/nodeand never have to update its resolutions? But I’m not super well-versed with package magic.