EventedConstructor incorrectly typed

[devpaul]

Evented isn't created with declare. It's a traditional es5 class function.

define(["./aspect", "./on"], function(aspect, on){
	// module:
	//		dojo/Evented

 	"use strict";
 	var after = aspect.after;
	function Evented(){
		// summary:
		//		A class that can be used as a mixin or base class,
		//		to add on() and emit() methods to a class
		//		for listening for events and emitting events:
		// example:
		//		|	define(["dojo/Evented", "dojo/_base/declare", "dojo/Stateful"
		//		|	], function(Evented, declare, Stateful){
		//		|		var EventedStateful = declare([Evented, Stateful], {...});
		//		|		var instance = new EventedStateful();
		//		|		instance.on("open", function(event){
		//		|		... do something with event
		//		|	 });
		//		|
		//		|	instance.emit("open", {name:"some event", ...});
	}
	Evented.prototype = {
		on: function(type, listener){
			return on.parse(this, type, listener, function(target, type){
				return after(target, 'on' + type, listener, true);
			});
		},
		emit: function(type, event){
			var args = [this];
			args.push.apply(args, arguments);
			return on.emit.apply(on, args);
		}
	};
	return Evented;
});

This needs fixing:

interface EventedConstructor extends _base.DeclareConstructor<Evented> {
		new (params?: Object): Evented;
	}

[dylans]

@mmckenziedev have you run into an issue with this yet?

[mmckenziedev]

@dylans I haven't had problems using Declare to mix this in to other classes.

I agree that technically this class is an ES5 class and shouldn't use DeclareConstructor. However, in practice, if you're using Declare to create a new class and you try to mix this in, if it is not a DeclareConstructor then it won't work properly due to the way Declare is defined. _base.d.ts#L620

If it's not breaking anything I would prefer to leave this as it is for now. When Typescript 2.2 comes out, and we can finally have a better expression syntax for mixins, then we could refactor the way Declare works.

https://github.com/Microsoft/TypeScript/wiki/Roadmap#22-february-2017 https://github.com/Microsoft/TypeScript/pull/13743

[devpaul]

Yeah, the pattern used with DeclareConstructor makes this a sticky issue to resolve.

It's usage should be OK with declare, but I was trying to extend Evented like a class

export default class<T> extends Evented implements StateMachineEvents<T> {

Which results in the error

error TS2510: Base constructors must all have the same return type.

The error may be due to Evented having two different constructors:

interface EventedConstructor extends _base.DeclareConstructor<Evented> {
	new (params?: Object): Evented;
}

interface DeclareConstructor<T> {
	new (...args: any[]): T & DeclareCreatedObject;

I was able to resolve by "fixing" the typings

export interface FixedEventedConstructor {
	new (params?: Object): dojo.Evented;
}
export const FixedEvented: FixedEventedConstructor = <any> Evented;

I haven't had a chance to play with TypeScript 2.2. Hopefully we'll be able to define mixin classes as purely ambient declarations that return functional constructors. If we have to apply it as a pattern in code and actually have to return a class that extends from a declare statement, I would be concerned about losing the metadata added by declare to allow for functional chaining via inherited. Hopefully it'll be the former and we'll have a good way of declaring define.

Anyway, all of this created a rabbit hole I knew I didn't want to go down on my own.

[schontz]

This issue bit me as well. Is https://github.com/dojo/typings/issues/113#issuecomment-278681008 the prescribed way around it for now?