Remove DOM lib reference in TypeScript types

[valeriangalliat]

Hi!

In backend projects, forcing the TypeScript compiler to import the DOM lib can causes issues, in particular shadowing of the Node.js or Cloudflare global types like Request.

This is somewhat of a known issue with TypeScript and packages that include the /// <reference lib="DOM" /> snippet break Node.js and other backend-only projects:

• https://stackoverflow.com/questions/63109147/using-typescript-without-dom-types-for-node
• Omitting "dom" from lib should enforce excluding lib.dom.d.ts (for nodejs) microsoft/TypeScript#43990
• [superagent] silently includes DOM typings in Node.js projects DefinitelyTyped/DefinitelyTyped#55382

superagent is typically a package that used to cause the same issue on server-side projects and it looks like they chose to remove the DOM lib reference as well

[isaacs]

I thought that this was needed to provide the AbortSignal type, but I guess not?

[valeriangalliat]

It looks like it was necessary until Node 15 which introduced a compatible built-in AbortSignal

So this means removing the /// <reference lib="DOM" /> as done in this PR seems that it could be an issue for TypeScript users:

• Using node<15
• That don't explicitly import the DOM lib
• And where none of their other dependencies import the DOM lib either

This might be something to consider when versioning this patch...

[glasser]

Yeah, we're running into trouble here — we have a library whose current major version needs to continue to work on Node 12 (though we're working on the new major version too which drops that), which means it ought to work with projects that use @types/node@12. This change breaks that.

Since TS types are structural, one option would be to just put the definition of AbortSignal (copied from the dom .d.ts) into your index.d.ts instead of relying on the global. That's basically analogous to the way you polyfill the implementation.

In fact, this would be an improvement anyway, because the @types/node AbortSignal definition is very incomplete! It doesn't even have addEventListener! There's a draft PR opened recently to flesh it out, but maybe just doing it yourself here would be better?

Would you take a PR to add a non-global definition of AbortSignal here?

[trevor-scheer]

@isaacs bump - ++ to what @glasser said, would you accept a PR that re-adds AbortSignal locally? I do agree this is a change in a good direction but it is breaking.