[types/node] Add promisify to util [wip] · Pull Request #16946 · DefinitelyTyped/DefinitelyTyped

ghost · 2017-06-04T04:06:14Z

A simple type definition that allows us to use the exciting util.promisify function.

Please fill in this template.

Use a meaningful title for the pull request. Include the name of the package modified.
Test the change in your own code. (Compile and run.)
Follow the advice from the readme.
Avoid common mistakes.
Run npm run lint package-name (or tsc if no tslint.json is present).

Select one of these and delete the others:

If changing an existing definition:

Provide a URL to documentation or source code which provides context for the suggested changes: https://nodejs.org/api/util.html#util_util_promisify_original
Increase the version number in the header if appropriate.
If you are making substantial changes, consider adding a tslint.json containing { "extends": "dtslint/dt.json" }.

I've never contributed before, and I'm new to Typescript, so please bear with me. Thanks!

A simple type definition that allows us to use the exciting util.promisify function.

dt-bot · 2017-06-04T04:06:16Z

types/node/index.d.ts

to authors (@DefinitelyTyped/DefinitelyTyped @parambirs @robdesideri @tellnes Microsoft TypeScript (account can't be detected)). Could you review this PR?
👍 or 👎?

Checklist

pass the Travis CI test?

hinell · 2017-06-05T13:00:39Z

There is issue for this PR #16860

tellnes · 2017-06-05T21:04:39Z

It looks correct to me, but it would be great if it somehow preserved the type information from the second argument of the callback by default.

Also, it is missing util.promisify.custom.

hinell · 2017-06-05T21:09:45Z

Also, it is missing util.promisify.custom.

👍

yuit · 2017-06-07T05:26:43Z

I will merge the PR and open a separate issue regarding missing util.promisify.custom

zh99998 · 2017-06-07T09:51:51Z

this wild Function definition will break any later definitely-typed benefits

have a look at @types/bluebird promisify type definition, it's such better

DefinitelyTyped/types/bluebird/index.d.ts

Lines 625 to 638 in 4daa341

    
             /** 
        
              * Returns a function that will wrap the given `nodeFunction`. Instead of taking a callback, the returned function will return a promise whose fate is decided by the callback behavior of the given node function. The node function should conform to node.js convention of accepting a callback as last argument and calling that callback with error as the first argument and success value on the second argument. 
        
              * 
        
              * If the `nodeFunction` calls its callback with multiple success values, the fulfillment value will be an array of them. 
        
              * 
        
              * If you pass a `receiver`, the `nodeFunction` will be called as a method on the `receiver`. 
        
              */ 
        
             static promisify<T>(func: (callback: (err: any, result: T) => void) => void, options?: Bluebird.PromisifyOptions): () => Bluebird<T>; 
        
             static promisify<T, A1>(func: (arg1: A1, callback: (err: any, result: T) => void) => void, options?: Bluebird.PromisifyOptions): (arg1: A1) => Bluebird<T>; 
        
             static promisify<T, A1, A2>(func: (arg1: A1, arg2: A2, callback: (err: any, result: T) => void) => void, options?: Bluebird.PromisifyOptions): (arg1: A1, arg2: A2) => Bluebird<T>; 
        
             static promisify<T, A1, A2, A3>(func: (arg1: A1, arg2: A2, arg3: A3, callback: (err: any, result: T) => void) => void, options?: Bluebird.PromisifyOptions): (arg1: A1, arg2: A2, arg3: A3) => Bluebird<T>; 
        
             static promisify<T, A1, A2, A3, A4>(func: (arg1: A1, arg2: A2, arg3: A3, arg4: A4, callback: (err: any, result: T) => void) => void, options?: Bluebird.PromisifyOptions): (arg1: A1, arg2: A2, arg3: A3, arg4: A4) => Bluebird<T>; 
        
             static promisify<T, A1, A2, A3, A4, A5>(func: (arg1: A1, arg2: A2, arg3: A3, arg4: A4, arg5: A5, callback: (err: any, result: T) => void) => void, options?: Bluebird.PromisifyOptions): (arg1: A1, arg2: A2, arg3: A3, arg4: A4, arg5: A5) => Bluebird<T>; 
        
             static promisify(nodeFunction: Function, options?: Bluebird.PromisifyOptions): Function;

ikokostya · 2017-06-07T11:02:09Z

Why this function was added to type definition for nodejs v7? util.promisify is available since v8.

hinell · 2017-06-07T11:31:26Z

@ikokostya What?

ikokostya · 2017-06-07T12:46:36Z

Current index.d.ts contains type definitions for Node.js v7

// Type definitions for Node.js v7.x

But util.promisify was added in Node.js v8 and is not available in Node.js 7

[kostya@arch ~]$ node -v
v7.10.0
[kostya@arch ~]$ node
> util.promisify
undefined

ikokostya · 2017-06-07T12:51:10Z

I think need to move current index.d.ts to v7/index.d.ts and make new index.d.ts for latest Node.js v8

blakeembrey · 2017-06-07T15:01:42Z

You should use something more like https://github.com/types/npm-thenify/blob/6ec63ea2de3f0e09d40dfccc9861f3aaa90b8853/index.d.ts#L19-L23 to keep the types. However, support for multiple arguments and the util.promisify.custom symbol is tricky and probably impossible without having it manually specified as a generic.

ghost · 2017-06-07T15:59:52Z

We should probably revert this as util.promisify is not in node v7.

hinell · 2017-06-07T16:03:01Z

@Andy-MS I think it's better to check out earlier index version and place it to a separate v7 folder as suggested by #17027

Of course if only we don't want to keep node/index in sync with most latest node version api.

ghost · 2017-06-07T20:09:03Z

@blakeembrey I think you're right, but that solution still limits the number of arguments the function can accept. I don't think util.promisify necessarily has to be used with the Node API functions, so there's no guarantee that there is any limit on the number of arguments. Note that util.promisify uses the spread operator.

blakeembrey · 2017-06-07T20:16:32Z

@cjhowe7 You can overload it up to 10, and then just do any[] for anything more than 10 or with multiple callback values. Those are more unlikely to occur and better typed than Function. Also, the spread operator shouldn't be relevant to the declaration - the return promise is still just as tricky because it's an object based on argument names and not an array (https://github.com/nodejs/node/blob/0d8021e5a4cf0a6aa3a700a361f6d42c2894f2ba/lib/internal/util.js#L235).

Edit: To clarify, typing it 100% perfectly is impossible. But we can get as close as possible by capturing the information that is available. Especially with default generics, we can do <T = any> and allow the user to have an escape hatch for defining the "real" return type manually.

[types/node] Add promisify to util [wip]

[types/node] Add promisify to util

9db0ab8

A simple type definition that allows us to use the exciting util.promisify function.

hinell approved these changes Jun 5, 2017

View reviewed changes

yuit merged commit 15e9249 into DefinitelyTyped:master Jun 7, 2017

yuit mentioned this pull request Jun 7, 2017

[Node] Missing util.promisify.custom #17015

Closed

ikokostya mentioned this pull request Jun 7, 2017

[node] Add definition for Node.js v8 #17027

Merged

8 tasks

KSXGitHub pushed a commit to KSXGitHub/DefinitelyTyped that referenced this pull request May 12, 2018

Merge pull request DefinitelyTyped#16946 from cjhowe7/patch-1

075e658

[types/node] Add promisify to util [wip]

Conversation

ghost commented Jun 4, 2017 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dt-bot commented Jun 4, 2017

Uh oh!

hinell commented Jun 5, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tellnes commented Jun 5, 2017

Uh oh!

hinell commented Jun 5, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yuit commented Jun 7, 2017

Uh oh!

zh99998 commented Jun 7, 2017

Uh oh!

ikokostya commented Jun 7, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hinell commented Jun 7, 2017

Uh oh!

ikokostya commented Jun 7, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ikokostya commented Jun 7, 2017

Uh oh!

blakeembrey commented Jun 7, 2017

Uh oh!

ghost commented Jun 7, 2017

Uh oh!

hinell commented Jun 7, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghost commented Jun 7, 2017

Uh oh!

blakeembrey commented Jun 7, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

ghost commented Jun 4, 2017 •

edited by ghost

Loading

hinell commented Jun 5, 2017 •

edited

Loading

hinell commented Jun 5, 2017 •

edited

Loading

ikokostya commented Jun 7, 2017 •

edited

Loading

ikokostya commented Jun 7, 2017 •

edited

Loading

hinell commented Jun 7, 2017 •

edited

Loading

blakeembrey commented Jun 7, 2017 •

edited

Loading