TypeScript Bindings

[delta62]

This pull request adds TypeScript bindings for the public interface of eventemitter3 (See issue #67).

The typings were written in the UMD style - that is, they will work correctly for scripts using module loaders as well as for scripts that reference EventEmitter as a global.

[lpinca]

Should exists be exists?? It is optional.

[delta62]

In TypeScript definitions, you can provide overloaded typings for the same method.

One implementation only takes event and always returns an array of listeners. The overload takes both parameters and either returns an array of listeners or a boolean, depending on the value of exists.

It is a best practice to put the most specific overload first, so I did swap the order of these in 365bef6.

[lpinca]

event is optional.

[delta62]

🔧 a711d93

[lpinca]

Can you please "fix" the indentation of these comments? Indent the second line and the following with 1 space.

[delta62]

Whoa.. good catch. That's weird. 🔧 3e5f97a

[lpinca]

Looks good but my experience with TypeScript is close to zero so if another pair of eyes could review this it would be great.

[lpinca]

cc: @Stubb0rn @roblav96

[Stubb0rn]

I believe module exports are not defined correctly. This is single export CommonJS module that exports non-module entity (class). This type of export cannot be defined using ES6 export syntax. You can take a look at discussion for PR that I created for this library in DefinitelyTyped repository DefinitelyTyped/DefinitelyTyped#10857

UPD. Nevermind, didn't notice related changes in #71

[lpinca]

@Stubb0rn so LGTY?

[Stubb0rn]

This property should be static

[lpinca]

Good catch, also the correct name is prefixed and can be either a string or a boolean (false).

[delta62]

🔧 9c00931

[Stubb0rn]

Instead of EventEmitter it is better to use this as return type of methods that support fluent interface. This will enable access to own methods of classes that extend EventEmitter:

class TestEmitter extends EventEmitter {
  ...
  test() {}
}

new TestEmitter().on().test();

[delta62]

🔧 780c427

[Stubb0rn]

Current definition of exports might work well with ES6 syntax but not so well with standard TypeScript import syntax:

import EventEmitter = require('eventemitter3');
new EventEmitter();

This code will not compile with current export definition and this syntax could be used by many users of this library that use it with TypeScript (especially in Node environment)

[delta62]

It's true that we cannot support both the import = and the ES6-style imports with the type definitions. We need to choose one. I put a bit of consideration into this when I wrote the defs.

The problem with import = is that if you use it anywhere in your code, you cannot compile into ES6 modules. I need to enable that setting, which was my motivation for this PR as well as #71.

Any users who are currently using this repository with TypeScript either wrote their own definitions or are using the (old) ones from DefinitelyTyped. I wonder what happens if this repository had these definitions included and the user had the DT definitions installed as well. That may allow legacy code to continue working while new projects can use the ES6 import syntax.

[delta62]

I don't think that shipping typings that use ES6-style imports will break anyone's existing code.

From TS design discussion:

If exists ambient external module named X {
  return the ambient external module 
}
else if X begins with './' or '../' or it is rooted path {
  try LOAD_AS_FILE(Y + X, loadOnlyDts=false)
  try LOAD_AS_DIRECTORY(Y + X, loadOnlyDts=false)
}
else {
  LOAD_NODE_MODULES(X, dirname(Y))
}
THROW "not found"

So if a user already has .d.ts files installed for eventemitter3, the definitions in this node module would not be used. This is consistent with what I tested locally.

[roblav96]

all looks good to me :D

[lpinca]

@delta62 @Stubb0rn @roblav96 thank you.