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

Add TypeScript definition #85

Closed
wants to merge 1 commit into from
Closed

Conversation

stroncium
Copy link

@stroncium stroncium commented Jul 15, 2021

Solves #74

  • works with generics(up to 6 arguments and 5 returned values)
  • works with anything except generics for any number of arguments
  • doesn't use include and exclude for typing, as we can't test RegExp while typing

IssueHunt Summary

Referenced issues

This pull request has been submitted to:


@sindresorhus
Copy link
Owner

Not sure whether it's possible to do this here, but did you consider using variadic tuple types? See: sindresorhus/p-all#9

index.test-d.ts Outdated Show resolved Hide resolved
@stroncium
Copy link
Author

Not sure whether it's possible to do this here, but did you consider using variadic tuple types? See: sindresorhus/p-all#9

My previous attempt to do something along the lines failed to handle generic functions and was typing everything generic as unknown, I found an issue from 2014 I think which kinda pointed me to it being impossible without overloads(though the catch all version in this branch inherits some of it, and works for everything except generics as far as I can tell). But I'll look into this implementation too in case I missed something.

@stroncium
Copy link
Author

Not sure whether it's possible to do this here, but did you consider using variadic tuple types? See: sindresorhus/p-all#9

@sindresorhus Worked out after some weird massage. Also cleaned up a bit.

@sindresorhus
Copy link
Owner

Many of the return types should be unknown instead of any to make it stricter. Unfortunately, all any's cannot be replaced with unknown in parameters because of TS special behavior of only any.

type R1 = 'R1';
type R2 = 'R2';
let someBoolean:boolean = Math.random() > 0.5;
const a1:A1 = 'A1';
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const a1:A1 = 'A1';
const a1: A1 = 'A1';

Applies in many other places

const a1:A1 = 'A1';
const a2:A2 = 'A2';

let function00 = (cb: (err: Error | undefined) => any) => { };
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let function00 = (cb: (err: Error | undefined) => any) => { };
let function00 = (callback: (error: Error | undefined) => any) => {};

Applies in many other places

}));

declare function generic10<A0>(a0:A0, cb:(error: Error | null | undefined) => any): any;
declare function generic01<R0>(cb:(error: Error | null | undefined, r0:R0) => any): any;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
declare function generic01<R0>(cb:(error: Error | null | undefined, r0:R0) => any): any;
declare function generic01<R0>(callback: (error: Error | null | undefined, r0: R0) => any): any;

declare function realLifeFunction1(url: string, callback: (error?: Error | null | undefined, response?: { statusCode: number }, body?: string) => void): void;
expectType<(url: string) => Promise<[{ statusCode: number }?, string?]>>(pify(realLifeFunction1, { multiArgs: true }))
declare function realLifeFunction2(url: string, callback: (response?: { statusCode: number }, body?: string) => void): void;
expectType<(url: string) => Promise<[{ statusCode: number }?, string?]>>(pify(realLifeFunction2, { multiArgs: true, errorFirst: false }))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
expectType<(url: string) => Promise<[{ statusCode: number }?, string?]>>(pify(realLifeFunction2, { multiArgs: true, errorFirst: false }))
expectType<(url: string) => Promise<[{statusCode: number}?, string?]>>(pify(realLifeFunction2, {multiArgs: true, errorFirst: false}))


expectAssignable<{
readFile: (path: string) => Promise<Buffer | undefined>,
}>(pify(fs, {exclude:['exists']}));
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
}>(pify(fs, {exclude:['exists']}));
}>(pify(fs, {exclude: ['exists']}));

/**
Returns a `Promise` wrapped version of the supplied function or module.
@param input - Callback-style function or module whose methods you want to promisify.
@returns Wrapped version of the supplied function or module.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
@returns Wrapped version of the supplied function or module.
@returns Wrapped version of the supplied function or module.

Applies in many other places

}

/**
Returns a `Promise` wrapped version of the supplied function or module.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Returns a `Promise` wrapped version of the supplied function or module.
Returns a `Promise` wrapped version of the supplied function or module.

T extends [...infer _, (infer R)?] ? R | undefined:
never;

type ExceptLast<T extends any[]> = T extends [ ...infer Head, any ] ? Head : any[];
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
type ExceptLast<T extends any[]> = T extends [ ...infer Head, any ] ? Head : any[];
type ExceptLast<T extends any[]> = T extends [...infer Head, any] ? Head : any[];

type LastParameter<T extends AnyFunction> = Last<Parameters<T>>;
type ParametersExceptLast<T extends AnyFunction> = ExceptLast<Parameters<T>>;

type SelectByOptionOr<O extends boolean, TTrue, TFalse> =
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The various utility types here need a short doc comment about what they do.


interface PifyOptions {
/**
By default, the promisified function will only return the second argument from the callback, which works fine for most APIs. This option can be useful for modules like `request` that return multiple arguments. Turning this on will make it return an array of all arguments from the callback, excluding the error argument, instead of just the second argument. This also applies to rejections, where it returns an array of all the callback arguments, including the error.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
By default, the promisified function will only return the second argument from the callback, which works fine for most APIs. This option can be useful for modules like `request` that return multiple arguments. Turning this on will make it return an array of all arguments from the callback, excluding the error argument, instead of just the second argument. This also applies to rejections, where it returns an array of all the callback arguments, including the error.
By default, the promisified function will only return the second argument from the callback, which works fine for most APIs. This option can be useful for modules like `request` that return multiple arguments. Turning this on will make it return an array of all arguments from the callback, excluding the error argument, instead of just the second argument. This also applies to rejections, where it returns an array of all the callback arguments, including the error.

@sindresorhus
Copy link
Owner

Are you sure this is the simplest way it could be done? It might be. I'm just hoping there are ways to further simplify it.

@sindresorhus
Copy link
Owner

@stroncium Bump :)

@sindresorhus
Copy link
Owner

Bump

@tom-sherman tom-sherman mentioned this pull request Dec 18, 2021
4 tasks
@tom-sherman
Copy link
Contributor

Are you sure this is the simplest way it could be done? It might be. I'm just hoping there are ways to further simplify it.

@sindresorhus I think it can be simplified quite a bit, I've made a start over on #86

@sindresorhus
Copy link
Owner

Closing in favor of #86

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

Successfully merging this pull request may close these issues.

3 participants