Add typings that work with current TS version

[haslers]

This branch basically just copies the typings from #47 and adds them. Works better than the current typings.

[AppVeyorBot]

❌ Build es6-promise-pool 203 failed (commit 17106e522d by @haslers)

[aMarCruz]

Fails with error TS2304: Cannot find name 'EventTarget'

[aMarCruz]

@haslers , this version is working for me and does not depend on external typings (it only exposes addEventListener & removeEventListener from EventTarget) :

/* eslint no-use-before-define:0, semi:0, space-infix-ops:0 */

declare namespace PromisePool {

  type EventName = 'fulfilled' | 'rejected';

  interface Options<T> {
    promise?: Promise<T>;
  }

  interface Event<T> {
    target: PromisePool<T>;
    type: EventName;
    data: T;
  }

  type Producer<T> = IterableIterator<Promise<T>> | Promise<T> | (() => PromiseLike<T> | null | void) | T;
  type Listener<T> = (param: Event<T>) => any;
}

declare class PromisePool<T> {

  constructor(
    source: () => PromisePool.Producer<T>,
    concurrency: number,
    options?: PromisePool.Options<T>
  );

  concurrency(concurrency: number): number;
  size(): number;
  active(): boolean;
  promise(): Promise<T> | null;
  start(): Promise<T>;

  addEventListener(type: PromisePool.EventName, listener: PromisePool.Listener<T>): void;
  removeEventListener(type: PromisePool.EventName, listener: PromisePool.Listener<T>): void;
}

export = PromisePool;

[Aankhen]

EventTarget is part of TypeScript’s definitions:

λ rg "interface EventTarget"
typescript\lib\lib.webworker.d.ts
526:interface EventTarget {

typescript\lib\lib.d.ts
8826:interface EventTarget {

typescript\lib\lib.es2016.full.d.ts
4674:interface EventTarget {

typescript\lib\lib.es2018.full.d.ts
4677:interface EventTarget {

typescript\lib\lib.dom.d.ts
4671:interface EventTarget {

typescript\lib\lib.es2017.full.d.ts
4679:interface EventTarget {

typescript\lib\lib.esnext.full.d.ts
4676:interface EventTarget {

typescript\lib\lib.es6.d.ts
10487:interface EventTarget {

What is your setup, @aMarCruz? If Promise is defined, EventTarget ought to be there too.

[aMarCruz]

@Aankhen , my tsconfig.json:

{
  "compilerOptions": {
    "lib": ["es6"],
    "module": "commonjs",
    "allowJs": true,
    "noImplicitReturns": true,
    "outDir": "lib",
    "sourceMap": true,
    "target": "es6"
  },
  "compileOnSave": true,
  "files": [
    "index.d.ts"
  ],
  "include": [
    "src"
  ]
}

from my package.json :

  "dependencies": {
    "es6-promise-pool": "^2.5.0",
    "firebase-admin": "~5.12.1",
    "firebase-functions": "^1.1.0"
  }

This is (almost) the default setup for Google Firebase cloud functions in a node.js environment, the files block with 'index.d.ts' was added by me.

[aMarCruz]

...my devDependencies:

  "devDependencies": {
    "eslint": "^5.1.0",
    "firebase-functions-test": "^0.1.2",
    "mocha": "^5.2.0",
    "ts-node": "^7.0.0",
    "typescript": "^2.9.2",
    "typescript-eslint-parser": "^16.0.1"
  },

and you're right, it is strange but TS can't find the EventTarget definition.
I'm using VS Code in Linux Mint and everything looks ok except EventTarget (Promise is defined).

[Aankhen]

Yeah, I copied your configuration and I get the same error unless I remove the "lib": ["es6"] in tsconfig.json, even though EventTarget exists in lib.es6.d.ts. That’s really strange.

EDIT:
Without even importing es6-promise-pool if I go to the definition of Promise in Emacs, it takes me to lib.es5.d.ts, which doesn’t have an EventTarget.

EDIT 2:
I got it working by specifying "lib": ["dom", "es6"].

[timdp]

Sorry about the radio silence here. I want to have another attempt at improving the typings. That said, I'm still not using TypeScript personally, and we've kind of drifted away from it at work as well.

I'm happy to accept working typings from the community. There are currently three PRs open around typings: #40, #47, and #63 (this one). Intuitively, I'd close the older two and merge the most recent one. However, that probably shouldn't be the only criterion.

Since I don't have a lot of skin in this game, I'm looking at you guys to review the PRs. Please let me know what you think. Thanks!

[Aankhen]

Apart from one observation about the options, I think this PR does a good job. I’ve been using it for a while. (I don’t have any exotic requirements, though!)

That said, if you’re not planning to use TypeScript here, then it would probably be better to remove the types from this repository and add them to DefinitelyTyped instead, which anyone can do.

[Aankhen]

This indicates that promise is an instance of Promise, whereas what it should be is a function that can produce a PromiseLike. I adapted a definition from Bluebird:

declare namespace PromisePool {
    interface PromiseFunction<A> {
        new(callback: (resolve: (value?: A) => void, reject: (reason?: any) => void) => void): PromiseLike<A>;
    }

    export interface Options<A> {
        promise?: PromiseFunction<A>
    }
}

Which made this work:

import PromisePool = require("es6-promise-pool");
import Bluebird = require("bluebird");

const options = { promise: Bluebird };
const pool = new PromisePool("test", 1, options);

const options2 = { promise: Promise };
const pool2 = new PromisePool("test", 1, options2);

[timdp]

Thanks for your feedback!

I think it would make a lot of sense to migrate the typings to DefinitelyTyped. I've just commited a change that removes them from the package, but I should probably hold off on releasing a new version until you or someone else has added them to DefinitelyTyped? What's the procedure?

[Aankhen]

Not at all. Thank you for a very useful library!

I can add them to DefinitelyTyped (it only needs writing a few tests to go with the definitions and opening a PR), but perhaps @aappddeevv would prefer to submit the definitions themselves (with the options fixed) so that their name is in the list of authors and they’re contacted in case of issues.

[timdp]

Sounds good!

After the typings have been added to DefinitelyTyped, tsc will automatically pick them up, right? Or do I need to put anything in package.json? If not, I can perhaps already do a minor release with the typings removed.

[Aankhen]

You don’t need to do anything; you only need a types entry if you include types. For DefinitelyTyped packages, the end-user runs npm install @types/package, e.g. npm install @types/es6-promise-pool. I would suggest waiting until that’s been added to do the release, because otherwise there will be a window where neither the package itself nor DefinitelyTyped has the types.

[timdp]

Makes sense. Could you ping me here once the typings are in DefinitelyTyped? Thanks!

[Aankhen]

I’ve been trying to put an improved version of the types together for DT since the original authors don’t seem to be around, but I ran into a roadblock: I haven’t been able to figure out how to avoid flattening the output type, i.e. tell TypeScript that if Bluebird is passed in as the promise library then the output is a Bluebird promise too. No luck on Stack Overflow so far. I’m hoping someone can eventually answer.

[Aankhen]

This is getting complicated. I realized my mistake, which solved the direct issue, but when you add another layer, there is a hole in the type inference that seems to make it impossible to express the types in TypeScript.