[node]: Base promisify signature on rest arguments and conditional types

[AlCalzone]

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.)
• Add or edit tests to reflect the change.
• Follow the advice from the readme.
• Avoid common mistakes.
• Run npm test <package to test>.

If changing an existing definition:

• Provide a URL to documentation or source code which provides context for the suggested changes: (see comment below)
• If this PR brings the type definitions up to date with a new version of the JS library, update the version number in the header.
• If you are making substantial changes, consider adding a tslint.json containing { "extends": "dtslint/dt.json" }. If for reason the any rule need to be disabled, disable it for that line using // tslint:disable-next-line [ruleName] and not for whole package so that the need for disabling can be reviewed.

I was asked to create this PR here based on the discussion in microsoft/TypeScript#41563 (comment)

@DanielRosenwasser mentioned that he isn't sure how to feel about the helper types - how to avoid leaking them into the global scope when the new type declarations are used?

I'm not sure if all the config changes are correct - lots of things to shuffle around just to add something for a newer TypeScript version.
And I'm not happy about is that the helper type Promisify is exposed as the return type - is there any way to resolve to the function signature it represents?

[typescript-bot]

@AlCalzone Thank you for submitting this PR!

This is a live comment which I will keep updated.

1 package in this PR

• node on npm, on unpkg
  • owner-approval: @ExE-Boss
  • Config files to check:
    • node/ts3.9/tsconfig.json: not the expected form
    • node/ts3.9/tslint.json: not the expected form

Code Reviews

Because this is a widely-used package, a DT maintainer will need to review it before it can be merged.

Status

• ❌ No merge conflicts
• ❌ Continuous integration tests have failed
• 🕐 A DT maintainer needs to approve changes which affect module config files

Once every item on this list is checked, I'll ask you for permission to merge and publish the changes.

Inactive

This PR has been inactive for 31 days — it is considered abandoned, and therefore closed!

---

Diagnostic Information: What the bot saw about this PR

{
  "type": "info",
  "now": "-",
  "pr_number": 49699,
  "author": "AlCalzone",
  "headCommitOid": "139c1679ecb5c2a475639582b7a312e4e7a1cde5",
  "lastPushDate": "2021-01-15T23:24:04.000Z",
  "lastActivityDate": "2021-04-02T09:04:01.000Z",
  "maintainerBlessed": false,
  "hasMergeConflict": true,
  "isFirstContribution": false,
  "tooManyFiles": false,
  "popularityLevel": "Critical",
  "pkgInfo": [
    {
      "name": "node",
      "kind": "edit",
      "files": [
        {
          "path": "types/node/base.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/package.json",
          "kind": "package-meta-ok"
        },
        {
          "path": "types/node/test/util.ts",
          "kind": "test"
        },
        {
          "path": "types/node/ts3.4/index.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/ts3.4/util.promisify.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/ts3.6/index.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/ts3.9/base.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/ts3.9/index.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/ts3.9/node-tests.ts",
          "kind": "test"
        },
        {
          "path": "types/node/ts3.9/tsconfig.json",
          "kind": "package-meta",
          "suspect": "not [the expected form](https://github.com/DefinitelyTyped/DefinitelyTyped#user-content-tsconfigjson)"
        },
        {
          "path": "types/node/ts3.9/tslint.json",
          "kind": "package-meta",
          "suspect": "not [the expected form](https://github.com/DefinitelyTyped/DefinitelyTyped#user-content-linter-tslintjson)"
        },
        {
          "path": "types/node/util.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/util.promisify.d.ts",
          "kind": "definition"
        }
      ],
      "owners": [
        "Microsoft",
        "DefinitelyTyped",
        "jkomyno",
        "alvis",
        "r3nya",
        "btoueg",
        "smac89",
        "touffy",
        "DeividasBakanas",
        "eyqs",
        "Hannes-Magnusson-CK",
        "KSXGitHub",
        "hoo29",
        "kjin",
        "ajafff",
        "islishude",
        "mwiktorczyk",
        "mohsen1",
        "n-e",
        "galkin",
        "parambirs",
        "eps1lon",
        "SimonSchick",
        "ThomasdenH",
        "WilcoBakker",
        "wwwy3y3",
        "samuela",
        "kuehlein",
        "bhongy",
        "chyzwar",
        "trivikr",
        "nguymin4",
        "yoursunny",
        "qwelias",
        "ExE-Boss",
        "Ryan-Willpower",
        "peterblazejewicz",
        "addaleax",
        "JasonHK",
        "victorperin",
        "ZYSzys"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Critical"
    }
  ],
  "reviews": [
    {
      "type": "approved",
      "reviewer": "ExE-Boss",
      "date": "2021-02-08T09:16:43.000Z",
      "isMaintainer": false
    },
    {
      "type": "stale",
      "reviewer": "rbuckton",
      "date": "2020-12-08T01:35:34.000Z",
      "abbrOid": "54b22fd"
    }
  ],
  "ciResult": "fail",
  "ciUrl": "https://github.com/DefinitelyTyped/DefinitelyTyped/commit/139c1679ecb5c2a475639582b7a312e4e7a1cde5/checks?check_suite_id=1839569975"
}

[typescript-bot]

🔔 @microsoft @DefinitelyTyped @jkomyno @a-tarasyuk @alvis @r3nya @btoueg @BrunoScheufler @smac89 @Touffy @DeividasBakanas @eyqs @Flarna @Hannes-Magnusson-CK @KSXGitHub @hoo29 @kjin @ajafff @islishude @mwiktorczyk @mohsen1 @n-e @galkin @parambirs @eps1lon @SimonSchick @ThomasdenH @WilcoBakker @wwwy3y3 @samuela @kuehlein @j-oliveras @bhongy @chyzwar @trivikr @nguymin4 @yoursunny @qwelias @ExE-Boss @Ryan-Willpower @peterblazejewicz @addaleax @JasonHK @victorperin @ZYSzys — please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

[typescript-bot]

@AlCalzone The CI build failed! Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

[typescript-bot]

@AlCalzone One or more reviewers has requested changes. Please address their comments. I'll be back once they sign off or you've pushed new commits or comments. If you disagree with the reviewer's comments, you can "dismiss" the review using GitHub's review UI. Thank you!

[AlCalzone]

You just need to override the promisify function.

That would be simpler, but wouldn't that just add another overload instead of replacing the existing ones?

[typescript-bot]

@ExE-Boss Thank you for reviewing this PR! The author has pushed new commits since your last review. Could you take another look and submit a fresh review?

[ExE-Boss]

You just need to override the promisify function.

That would be simpler, but wouldn't that just add another overload instead of replacing the existing ones?

You can do what’s being done with the globals.global.d.ts file to provide different definitions of declare var global between TypeScript 3.7+ and TypeScript ≤3.6 by utilising util.promisify.d.ts.

[AlCalzone]

@ExE-Boss I've tried what you suggested - hope it is correct this time. All the interdependencies between the different types-versions are confusing.

I've also solved the problem that promisify returned Promisify<...> instead of the function signature type.

[ExE-Boss]

This file and directory is probably unnecessary, since util.promisify.d.ts doesn’t differ between ts3.4 and ts3.9.

[AlCalzone]

Is there a way to limit the signature to TS 4.0+ without it?

[AlCalzone]

I thought the ts3.6 directory exists because TS 3.7 added some new features that are being used. This should be the same - new feature starting with TS 4.0?

[rbuckton]

I think the point @AlCalzone is trying to make here is that ts3.4/util.promisify.d.ts isn't included in the set of referenced files (it is referenced by ts3.6/index.d.ts but not by ts3.6/base.d.ts).

[AlCalzone]

Honestly, I'm not sure what to do here.
There are some existing definitions that should be active for TS >= 3.7 (thus the 3.6 directory).
Then there are the new promisify signatures which should be active for TS >= 4.0.
And there are the old promisify signatures which need to be available for all versions <= 3.9.

I got it to work the way it is now, but if this is not the correct way, I'm happy to change it.

[AlCalzone]

@rbuckton @ExE-Boss I think everything should be addressed now

[ankon]

The PR will be closed on Mar 10th (in a week) if the issues aren't addressed.

Out of interest: Where would one report issues with the bot seemingly not being able to read a calendar?

[AlCalzone]

Mar 10th (in a week)

Hah, good old JavaScript with its 0-based monts 😏

[jablko]

@ankon I opened DefinitelyTyped/dt-mergebot#364 to fix the calendar bug you spotted, which is my fault. Sorry about that!

[typescript-bot]

@AlCalzone I haven't seen any activity on this PR in more than three weeks, and it still has problems that prevent it from being merged. The PR will be closed on Mar 12th (in a week) if the issues aren't addressed.

[AlCalzone]

Which issues though? This is waiting for a re-review.

[AlCalzone]

@rbuckton got time for a re-review?

[typescript-bot]

@AlCalzone Unfortunately, this pull request currently has a merge conflict 😥. Please update your PR branch to be up-to-date with respect to master. Have a nice day!

[AlCalzone]

🤷🏻‍♂️

[typescript-bot]

@AlCalzone I haven't seen any activity on this PR in more than three weeks, and it still has problems that prevent it from being merged. The PR will be closed on Apr 8th (in a week) if the issues aren't addressed.

[AlCalzone]

I don't care anymore. I was asked by a TS developer to raise this PR, yet I can't get anyone to review it and now I need to start from scratch because of the merge conflicts.

Do what you want with it, I'll ignore this thread now.

[typescript-bot]

@AlCalzone I haven't seen any activity on this PR in more than three weeks, and it still has problems that prevent it from being merged. The PR will be closed on May 2nd (in a week) if the issues aren't addressed.

[typescript-bot]

@AlCalzone To keep things tidy, we have to close PRs that aren't mergeable and don't have activity in the last month. No worries, though — please open a new PR if you'd like to continue with this change. Thank you!

[OmgImAlexis]

@AlCalzone don't mean to annoy you with this ping but is there any chance I could assist in getting this re-opened and merged? This is the first time I've managed to find a working version of promisify with typescript.

[AlCalzone]

@OmgImAlexis if you like, you can try to put up an up-to-date PR with my changes. I'll gladly assist if there are any problems with my types, but I don't have the capacity to jump through these hoops again.