Remove static abort from AbortSignal

[sandersn]

@types/node doesn't have it, and it's not worth it to make @types/node create a new typingsVersion.

[github-actions]

Thanks for the PR!

This section of the codebase is owned by @saschanaz - if they write a comment saying "LGTM" then it will be merged.

[saschanaz]

I think this one should be preferred over node, as Node is just following the web standard? What happened there?

[SimonSchick]

But node does have this method?

Type ".help" for more information.
> AbortSignal.abort()
AbortSignal { aborted: true }

[saschanaz]

But node does have this method?

Web browsers and Node.js have the method defined as https://dom.spec.whatwg.org/#interface-AbortSignal .

[orta]

Yeah, agree - its more that we didn't think adding this was worth forcing @types/node to have to switch to TS 4.5 for their types and then requiring many projects to update for this one property. Not that it's incorrect

[saschanaz]

Can it be fixed locally in TS rather than affecting @types/web?

[orta]

It's a normal assignability bug, not a change in TypeScript (you can see a tonne in here)

node_modules/@types/node/globals.d.ts(85,13): error TS2403: Subsequent variable declarations must have the same type.  Variable 'AbortSignal' must be of type '{ new (): AbortSignal; prototype: AbortSignal; abort(): AbortSignal; }', but here has type '{ new (): AbortSignal; prototype: AbortSignal; }'.

Basically it means that anyone with a current or older version of @types/node cannot upgrade to TS 4.5 without updating their node types, and it means that the @types/node would need to bump their minimum TS version to 4.5 for this one property.

Which to me isn't a reasonable trade-off for that extra definition

[saschanaz]

I mean, we'll have to add that to node one day, so it would be good if TS have a local patch that removes abort() so that anyone need it can install @types/web?

[saschanaz]

That can happen in deploy script IMO.

[SimonSchick]

Can we not change the type of AbortSignal to be an interface and use declaration merging?

[saschanaz]

Can we not change the type of AbortSignal to be an interface and use declaration merging?

Sounds like an option tbh.

[sandersn]

Does the code have the ability to generate an interface for the static side too? If not, I guess we could add that.

[saschanaz]

No such ability, I think AbortSignal can be special cased.

[sandersn]

oh, I forgot about addedTypes.jsonc. That should work.

[orta]

So, I think we're going to have to take this PR as-is because I don't think we're going to get a good answer which doesn't force users to update node or force a typesVersion on node before the release today. We shipped a build to TypeScript with these changes in, because we don't want to force the node update on anyone migrating to the new TS build, and I want to have corresponding @types/web to refer to it.

I'll make a PR reverting it after we're sure the RC dom is going to be what we ship in TS.

I brought up microsoft/TypeScript#43972 in a TS language design meeting on Wednesday and there was general agreement to explore finding the common subset and extract it in here which is going to probably be tricky work in this repo. microsoft/TypeScript#45328

[saschanaz]

I want to have corresponding @types/web to refer to it.

I'd prefer manual patching in deploy script if this isn't a strict requirement.

[orta]

It's not, I can do that

[saschanaz]

Looks good, please reformat 😉

[saschanaz]

LGTM

[github-actions]

Sorry @saschanaz, you don't have access to merge: /deploy/createTypesPackages.js.

[saschanaz]

This maaayy advertise @types/web but your call really.

[orta]

OK, let's get this in - which will unblock merging some of the other PRs