dojo / typings

Dojo 1 - TypeScript Typings (including Dijit and DojoX)
Other
28 stars 37 forks source link

EventedConstructor has two constructors of different types #170

Closed HeikoStudt closed 3 years ago

HeikoStudt commented 4 years ago

Using a current typescript transpiler (^3.9.7), we got the error

TS2510 --- Base constructors must all have the same return type.

This is due to EventedConstructor defines the constructor as new (params?: Object): Evented; but the base class _base.DeclareConstructor has one defined by: new (...args: any[]): Evented & DeclareCreatedObject;

Therefore, Evented could not be used as a base class; I will attach an PR

HeikoStudt commented 4 years ago

@msssk Can you please approve this PR - it is the same as the other with the hickup.

msssk commented 4 years ago

@HeikoStudt where are you seeing this? I can't find it in dojo/typings:

new (...args: any[]): Evented & DeclareCreatedObject

Can you add an explanatory comment to the PR? It's an awkward scenario we are in where the existing typings are accurate, but don't work in practice. dojo/Evented is a plain JS constructor function, but it is intended to be used with dojo/_base/declare and declare supports mixing in plain JS constructors. Specifying Evented & _base.DeclareCreatedObject as the return type of the Evented constructor is not accurate and could actually allow invalid code to pass type-checking. I feel like that is a lesser danger than the problem of getting type errors when trying to use modules created with declare that include dojo/Evented though. So if you can add a comment to the PR I think we are good to go:

interface EventedConstructor extends _base.DeclareConstructor<Evented> {
    // While not technically accurate, it is necessary to add `_base.DeclareCreatedObject`
    // to avoid errors when mixing in Evented using declare
    // see https://github.com/dojo/typings/issues/170
    new (params?: Object): Evented & _base.DeclareCreatedObject;
}
HeikoStudt commented 4 years ago

Probably, I made a substitution while putting it here. It is from dojo/1.11/_base.d.ts:

new (...args: any[]): T & DeclareCreatedObject;

I put the comment into the PR, thanks a lot!

edhager commented 3 years ago

I landed the PR