microsoft / TypeScript-DOM-lib-generator

Tool for generating dom related TypeScript and JavaScript library files
Apache License 2.0
616 stars 418 forks source link

Not all interfaces should be marked as constructable #1162

Open mykhalov opened 2 years ago

mykhalov commented 2 years ago

E.g. trying to call AbortSignal with new throws Illegal constructor error.

> new AbortSignal()
  Uncaught TypeError: Illegal constructor.

This is not currently handled by the declaration:

declare var AbortSignal: {
    prototype: AbortSignal;
    new(): AbortSignal;
};
saschanaz commented 2 years ago

This is because instanceof check requires a constructor member. But never seemingly works:

interface Foo {}

declare var Foo: {
    prototype: Foo;
    new(): never;
};

let foo = {} as Foo;
if (foo instanceof Foo) {
  foo // $Foo
}

@orta does new(): never make sense to fix this?

orta commented 2 years ago

We bounced the idea back and forth a bit, and generally think this is more likely to cause more breakages than it cures 👍🏻

saschanaz commented 2 years ago

cause more breakages than it cures

What breakages for example?

aloisklink commented 2 years ago

This also affects NodeList (which could have prevented a bug: https://github.com/mermaid-js/mermaid/pull/3396)

Maybe it's worth putting in a /** @deprecated */ JSDoc tag to constructors that throw Illegal constructor, so some tools (like ESLint/VS Code) will warn about using them.

saschanaz commented 2 years ago

Sounds good to me but I wonder what TS team thinks about the suggestion, maybe @DanielRosenwasser?

Skalman commented 4 months ago

I just ran into this issue. Adding /** @deprecated */ seems like a good and simple fix.

What do you think, @DanielRosenwasser?

saschanaz commented 5 days ago

@sandersn Do you some context around https://github.com/microsoft/TypeScript-DOM-lib-generator/issues/1162#issuecomment-933853993 ?