Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The bottleneck type definitions? #26

Closed
romelgomez opened this issue Nov 9, 2016 · 13 comments
Closed

The bottleneck type definitions? #26

romelgomez opened this issue Nov 9, 2016 · 13 comments

Comments

@romelgomez
Copy link

romelgomez commented Nov 9, 2016

https://www.npmjs.com/package/@types/bottleneck

https://www.typescriptlang.org/docs/handbook/declaration-files/introduction.html

First attempt:

// Type definitions for twit 1.15.0S
// Project: https://github.com/SGrondin/bottleneck
// Definitions by: Romel Gomez <https://github.com/romelgomez>
// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped


declare module 'bottleneck' {

  namespace Bottleneck {
    export interface Fn {
      (...args: any[]): any
    }
  }

  class Bottleneck {

    /**
     * @see https://github.com/SGrondin/bottleneck#constructor
     */
    constructor(maxNb?: number, minTime?: number, highWater?: number, strategy?: number, rejectOnDrop?: boolean);

    /**
     * @see https://github.com/SGrondin/bottleneck#submit
     */
    submit(fn: Bottleneck.Fn, ...args: any[]);

    /**
     * @see https://github.com/SGrondin/bottleneck#schedule
     */
    schedule(fn: Bottleneck.Fn, ...args: any[]);

    /**
     * @see https://github.com/SGrondin/bottleneck#submitpriority
     */
    submitPriority(priority:number, fn: Bottleneck.Fn, ...args: any[]);

    /**
     * @see https://github.com/SGrondin/bottleneck#schedulepriority
     */
    schedulePriority(priority:number, fn: Bottleneck.Fn, ...args: any[]);

  }

  export = Bottleneck;

}
@SGrondin
Copy link
Owner

Hi. Yeah, a few people have attempted to write TypeScript type definitions before: Tattoo, Halo-client, but none are complete or perfect.

The main issue I can see is that everybody uses (abuses) any instead of properly encoding the generic signature.

Just check that Tattoo link, you can see that they properly encoded the schedule function. If you pass a PromiseProducer<T> then schedule will return a Promise<T>. This type checks that your types match: the type of the promise producer and the promise returned by bottleneck will always be the same.

But there's a problem with those Tattoo type definitions, you can also see that they use any for the arguments to the PromiseProducer instead of using generics to make them match the schedule arguments. Honestly, I don't even know if TypeScript's type system is expressive enough to encode this, but it's something I want to make sure of before adding any incomplete or weak type definitions to this project.

If you want to make your type definitions generic and add all the missing methods, I'll be super happy to merge it and maintain it. Otherwise, I'll probably do it myself at some point in December.

Pinging @garlicnation and @usergenic in case they have comments about this.

@usergenic
Copy link

@SGrondin I think we did that because the arguments to the scheduled function aka PromiseProducer were deemed unknown. Is it invoked with arguments? I don't see how they should be related to <T>?

@usergenic
Copy link

O_o Oh I understand your point now-- You're asking why we don't relate a specific type for A1 to the first value of args of the PromiseProducer and A2 to the second etc. Yeah I don't think there's a mechanism to make a type PromiseProducer<T>(<A>) such that <A> could define schedule<T>(promise: PromiseProducer<T>(<A>), ...args: A) though I might try that.

@SGrondin
Copy link
Owner

SGrondin commented Nov 19, 2016

Yes @usergenic you understand my point. If this were an ML language (like Haskell, SML or OCaml), I could do this:

val schedule: ('a -> 'b -> 't promise) -> 'a -> 'b -> 't promise

In other words, given:

  • A function. Let's call it FN, it takes any 2 generic arguments and returnes a promise of type t.
  • An argument matching the type of the first argument of FN
  • An argument matching the type of the second argument of FN.

Then schedule() will return a promise of the same type as the promise returned by FN.

Instead of using an interface for the PromiseProducer, is there a way to tell TypeScript that an argument is a function (and then defining the argument of that function)? That would solve the problem.

@usergenic
Copy link

Typescript doesn't seem to like the idea of PromiseProducer( ...args: A)

Tried this:

    schedule<T,A>(promise: (...args: A) => Promise<T>, ...args: A):
        Promise<T>;

Got this:

schedule<T,A>(promise: (...args: A) => Promise<T>, ...args: A):
                              ~~~~~~~~~~

custom_typings/bottleneck.d.ts(4,29): error TS2370: A rest parameter must be of an array type.

And it acts weird if you try to do this:

    schedule<T,A>(promise: (...args: A[]) => Promise<T>, ...args: A[]):
        Promise<T>;

When arg types differ:

102         return await this._cloneRateLimiter
                         ~~~~~~~~~~~~~~~~~~~~~~
103             .schedule((a: string, b: number) => openRepo.fetchAll(this._cloneOptions.fetchOpts), 'hi', 2)
    ~~~~~~~~~~~~~~~~~~~~~

src/git.ts(102,22): error TS2453: The type argument for type parameter 'A' cannot be inferred from the usage. Consider specifying the type arguments explicitly.
  Type argument candidate 'string' is not a valid type argument because it is not a supertype of candidate 'number'.

It can't seem to infer that A can be a type representing multiple types. So yeah not clear if there's a way to get current typescript to pattern match type sequence in args like that.

@SGrondin
Copy link
Owner

SGrondin commented Dec 6, 2016

Thanks for trying it out. I'll give it a shot over the holidays.

@alexperovich
Copy link
Contributor

I too want definitions for this.

Typescript's type system doesn't have any way to express these functions in the general case for any number of arguments, however you can write overloaded function definitions for variants with different numbers of arguments like this:

  schedule<R>(fn: () => PromiseLike<R>): Promise<R>;
  schedule<R, A1>(fn: (arg1: A1) => PromiseLike<R>, arg1: A1): Promise<R>;
  schedule<R, A1, A2>(fn: (arg1: A1, arg2: A2) => PromiseLike<R>, arg1: A1, arg2: A2): Promise<R>;

This can be repeated up to some reasonable limit of argument count.

@SGrondin
Copy link
Owner

@alexperovich that's great! If you're interested in doing it, I'll very be happy to review and merge your PR.

@jamesandersonwalsh
Copy link

@SGrondin, hey I was wondering if you were planning on publishing this change to NPM anytime soon? Would love to have types for this library without having to pull down the module from Github.

@SGrondin
Copy link
Owner

@jimboslicethat I'll be releasing it tonight or tomorrow night!

@SGrondin
Copy link
Owner

@jimboslicethat @KaidenR Version 1.16.0 with type definitions has been released!

@mrns
Copy link

mrns commented May 10, 2019

Can anyone specify how these types are supposed to be installed? I am trying typescript in a project and installing @types/bottleneck as I did with other packages does not work, getting 404. Thanks!

@SGrondin
Copy link
Owner

The types are part of the Bottleneck module itself, all you need to do is npm install bottleneck and you're good to go!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants