Improve TypeScript types by using variadic tuple instead of overloads by privatenumber · Pull Request #9 · sindresorhus/p-all

privatenumber · 2021-05-30T05:14:17Z

Problem

Function signature was using overloads to hardcode support for up to 10 cases

Changes

Use variadic tuple types to support all input lengths.

This is a breaking change in type:

Before:
```
pAll<string, number>([p1, p2]);
```
With variadic tuple:
```
pAll<[string, number]>([p1, p2]);
```
Tests are passing

References

Variadic tuple in Promise.all issue 4.0: Cannot use variadic tuples in Promise.all microsoft/TypeScript#39788
Variadic tuple to Promise.all experiment Add variadic tuple overload to Promise.all microsoft/TypeScript#39796

index.d.ts

privatenumber · 2021-05-30T19:34:10Z

Looks like tsd was outdated and didn't support the variadic tuples syntax. Works fine now.

privatenumber · 2021-05-30T20:36:37Z

Looks like one of tsd@0.11.0's dependencies was previously satisfying a typescript dependency required by @typescript-eslint in xo but is no longer satisfied:

Cannot find module 'typescript'

Error: Failed to load parser '/home/runner/work/p-all/p-all/node_modules/@typescript-eslint/parser/dist/parser.js' declared in 'BaseConfig': Cannot find module 'typescript'
Require stack:
- /home/runner/work/p-all/p-all/node_modules/@typescript-eslint/typescript-estree/dist/parser.js
- /home/runner/work/p-all/p-all/node_modules/@typescript-eslint/parser/dist/parser.js
- /home/runner/work/p-all/p-all/node_modules/eslint/lib/cli-engine/config-array-factory.js
- /home/runner/work/p-all/p-all/node_modules/eslint/lib/cli-engine/cascading-config-array-factory.js
- /home/runner/work/p-all/p-all/node_modules/eslint/lib/cli-engine/cli-engine.js
- /home/runner/work/p-all/p-all/node_modules/eslint/lib/cli-engine/index.js
- /home/runner/work/p-all/p-all/node_modules/eslint/lib/api.js
- /home/runner/work/p-all/p-all/node_modules/xo/index.js
- /home/runner/work/p-all/p-all/node_modules/xo/cli-main.js
- /home/runner/work/p-all/p-all/node_modules/xo/cli.js
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:885:15)
    at Function.Module._load (internal/modules/cjs/loader.js:730:27)
    at Module.require (internal/modules/cjs/loader.js:957:19)
    at require (internal/modules/cjs/helpers.js:88:18)
    at Object.<anonymous> (/home/runner/work/p-all/p-all/node_modules/@typescript-eslint/typescript-estree/dist/parser.js:20:25)
    at Module._compile (internal/modules/cjs/loader.js:1068:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1097:10)
    at Module.load (internal/modules/cjs/loader.js:933:32)
    at Function.Module._load (internal/modules/cjs/loader.js:774:14)
    at Module.require (internal/modules/cjs/loader.js:957:19)
npm ERR! Test failed.  See above for more details.

Installing the latest typescript as a devDependency yields:

Parsing error: Cannot read property map of undefined

$ npm test

> p-all@3.0.0 test /Users/osame/Documents/public-github/sindresorhus/p-all
> xo && ava && tsd

  index.test-d.ts
  ✖  Parsing error: Cannot read property map of undefined

  index.d.ts
  ✖  Parsing error: Cannot read property map of undefined

  2 errors

Looks like an internal error within xo, perhaps an incompatibility between @typescript-eslint/parser and the latest typescript. There's no expected version of typescript specified in their package.json.

Upgrading xo to the latest yields:

New linting errors

$ npm test

> p-all@3.0.0 test /Users/osame/Documents/public-github/sindresorhus/p-all
> xo && ava && tsd

  index.test-d.ts:2:15
  ✖   2:15   A require() style import is forbidden.                                 @typescript-eslint/no-require-imports
  ✖  16:7    Unsafe assignment of an any value.                                     @typescript-eslint/no-unsafe-assignment
  ✖  18:20   Unsafe member access [0] on an any value.                              @typescript-eslint/no-unsafe-member-access
  ✖  19:20   Unsafe member access [1] on an any value.                              @typescript-eslint/no-unsafe-member-access
  ✖  20:18   Unsafe member access [2] on an any value.                              @typescript-eslint/no-unsafe-member-access
  ✖  21:20   Unsafe member access [3] on an any value.                              @typescript-eslint/no-unsafe-member-access

  index.d.ts:34:15
  ⚠  10:1    Unexpected todo comment: TODO: Refactor the whole definition back....  no-warning-comments
  ✖  34:15   pAll is already defined.                                               @typescript-eslint/no-redeclare
  ✖  37:15   There should be no space after {.                                      @typescript-eslint/object-curly-spacing
  ✖  37:110  There should be no space before }.                                     @typescript-eslint/object-curly-spacing

  index.js:1:1
  ✖   1:1    Do not use "use strict" directive.                                     unicorn/prefer-module
  ✖   2:14   Do not use "require".                                                  unicorn/prefer-module
  ✖   4:1    Do not use "module".                                                   unicorn/prefer-module

  1 warning
  12 errors

I would have to add a tsconfig.json to eliminate the @typescript-eslint/no-unsafe-member-access errors. I can copy the default config that tsd uses, but I know you usually don't have these in your repo. I can disable the prefer-module rules for now.

How would you like me to move forward?

sindresorhus · 2021-05-31T07:16:43Z

I'll handle the linting stuff.

refactor(type): use variadic-tuple to remove overloads

ed11a17

privatenumber commented May 30, 2021

View reviewed changes

index.d.ts Outdated Show resolved Hide resolved

privatenumber added 2 commits May 30, 2021 05:42

fix: object map to check whether function

8cc5652

style: lintfix

195e514

privatenumber marked this pull request as ready for review May 30, 2021 09:44

chore: upgrade tsd

6babf02

sindresorhus changed the title ~~refactor(type): use variadic-tuple to remove overloads~~ Improve TypeScript types by using variadic tuple instead of overloads May 31, 2021

sindresorhus merged commit ea9c277 into sindresorhus:main May 31, 2021

privatenumber mentioned this pull request May 31, 2021

Add .allSettled() method sindresorhus/p-progress#10

Closed

Richienb mentioned this pull request Jun 1, 2021

Improve TypeScript definition sindresorhus/p-progress#22

Merged

sindresorhus mentioned this pull request Jul 16, 2021

Add TypeScript definition sindresorhus/pify#85

Closed

privatenumber deleted the variadic-tuple-type branch December 9, 2021 21:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Improve TypeScript types by using variadic tuple instead of overloads#9

Improve TypeScript types by using variadic tuple instead of overloads#9
sindresorhus merged 4 commits intosindresorhus:mainfrom
privatenumber:variadic-tuple-type

privatenumber commented May 30, 2021 •

edited

Loading

Uh oh!

Uh oh!

privatenumber commented May 30, 2021

Uh oh!

privatenumber commented May 30, 2021

Uh oh!

sindresorhus commented May 31, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

privatenumber commented May 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Changes

References

Uh oh!

Uh oh!

privatenumber commented May 30, 2021

Uh oh!

privatenumber commented May 30, 2021

Uh oh!

sindresorhus commented May 31, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

privatenumber commented May 30, 2021 •

edited

Loading