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

std/node: add util.callbackify #5415

Merged
merged 1 commit into from
May 20, 2020

Conversation

EvanHahn
Copy link
Contributor

@EvanHahn EvanHahn commented May 14, 2020

This adds Node's util.callbackify to std/node/util.ts.

I lifted most of this from the original Node source code (and its tests). I tried to make minimal modifications to the source.

I made a few arbitrary decisions:

  • I was unable to do the function's types elegantly. I made overloads for functions that have 0 to 5 (inclusive) arguments, excluding the callback. I would love to know a better way to do this. (It seems that the folks at DefinitelyTyped were also stumped, though maybe their solution is deliberate.)
  • There are a few edge cases that cause custom Node errors to be produced. Instead of re-implementing those errors completely, I created simplified classes. These are mostly correct but are not identical to the real Node errors.
  • The tests implement a possibly-arcane TestQueue class. I originally used a lot of inline promises but found it too repetitive.

Closes #5366.

@EvanHahn
Copy link
Contributor Author

Looks like I've introduced some linting errors. I'll fix those.

@EvanHahn EvanHahn marked this pull request as draft May 14, 2020 19:51
@EvanHahn EvanHahn force-pushed the std-node-util-callbackify branch 2 times, most recently from 69ce6bb to 7223ed5 Compare May 14, 2020 21:42
@EvanHahn EvanHahn marked this pull request as ready for review May 14, 2020 21:56
Copy link
Member

@ry ry left a comment

Choose a reason for hiding this comment

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

Thanks - looks good!

std/node/_util/_util_callbackify.ts Outdated Show resolved Hide resolved
std/node/util.ts Show resolved Hide resolved
This adds [Node's `util.callbackify`][0] to `std/node/util.ts`.

I lifted most of this from the [original Node source code][1] (and [its
tests][2]). I tried to make minimal modifications to the source.

I made a few arbitrary decisions:

- I was unable to do the function's types elegantly. I made overloads
  for functions that have 0 to 5 (inclusive) arguments, excluding the
  callback. I would love to know a better way to do this. (It seems that
  the folks at DefinitelyTyped [were also stumped][3], though maybe
  their solution is deliberate.)
- There are a few edge cases that cause custom Node errors to be
  produced. Instead of re-implementing those errors completely, I
  created simplified classes. These are mostly correct but are not
  identical to the real Node errors.
- The tests implement a possibly-arcane `TestQueue` class. I originally
  used a lot of inline promises but found it too repetitive.

Closes [denoland#5366][4].

[0]: https://nodejs.org/api/util.html#util_util_callbackify_original
[1]: https://github.com/nodejs/node/blob/47804933012841f2dc90626bdcc161adf34569a5/lib/util.js#L183-L226
[2]: https://github.com/nodejs/node/blob/47804933012841f2dc90626bdcc161adf34569a5/test/parallel/test-util-callbackify.js
[3]: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/7d24857ddb07ab637dfa8c477d13352f8a8206fc/types/node/util.d.ts#L61-L84
[4]: denoland#5366
@EvanHahn EvanHahn force-pushed the std-node-util-callbackify branch from 7223ed5 to d00e2bc Compare May 15, 2020 14:49
@bartlomieju bartlomieju added this to the std/0.52.0 milestone May 18, 2020
Copy link
Member

@ry ry left a comment

Choose a reason for hiding this comment

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

LGTM - thanks @EvanHahn

@ry ry merged commit f5c0188 into denoland:master May 20, 2020
EvanHahn added a commit to EvanHahn/evanhahn-dot-com that referenced this pull request May 20, 2020
caspervonb pushed a commit to caspervonb/deno_std that referenced this pull request Jan 21, 2021
This adds [Node's `util.callbackify`][0] to `std/node/util.ts`.

I lifted most of this from the [original Node source code][1] (and [its
tests][2]). I tried to make minimal modifications to the source.

I made a few arbitrary decisions:

- I was unable to do the function's types elegantly. I made overloads
  for functions that have 0 to 5 (inclusive) arguments, excluding the
  callback. I would love to know a better way to do this. (It seems that
  the folks at DefinitelyTyped [were also stumped][3], though maybe
  their solution is deliberate.)
- There are a few edge cases that cause custom Node errors to be
  produced. Instead of re-implementing those errors completely, I
  created simplified classes. These are mostly correct but are not
  identical to the real Node errors.
- The tests implement a possibly-arcane `TestQueue` class. I originally
  used a lot of inline promises but found it too repetitive.

Closes [denoland/deno#5366][4].

[0]: https://nodejs.org/api/util.html#util_util_callbackify_original
[1]: https://github.com/nodejs/node/blob/47804933012841f2dc90626bdcc161adf34569a5/lib/util.js#L183-L226
[2]: https://github.com/nodejs/node/blob/47804933012841f2dc90626bdcc161adf34569a5/test/parallel/test-util-callbackify.js
[3]: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/7d24857ddb07ab637dfa8c477d13352f8a8206fc/types/node/util.d.ts#L61-L84
[4]: denoland/deno#5366
caspervonb pushed a commit to caspervonb/deno_std that referenced this pull request Jan 24, 2021
This adds [Node's `util.callbackify`][0] to `node/util.ts`.

I lifted most of this from the [original Node source code][1] (and [its
tests][2]). I tried to make minimal modifications to the source.

I made a few arbitrary decisions:

- I was unable to do the function's types elegantly. I made overloads
  for functions that have 0 to 5 (inclusive) arguments, excluding the
  callback. I would love to know a better way to do this. (It seems that
  the folks at DefinitelyTyped [were also stumped][3], though maybe
  their solution is deliberate.)
- There are a few edge cases that cause custom Node errors to be
  produced. Instead of re-implementing those errors completely, I
  created simplified classes. These are mostly correct but are not
  identical to the real Node errors.
- The tests implement a possibly-arcane `TestQueue` class. I originally
  used a lot of inline promises but found it too repetitive.

Closes [denoland/deno#5366][4].

[0]: https://nodejs.org/api/util.html#util_util_callbackify_original
[1]: https://github.com/nodejs/node/blob/47804933012841f2dc90626bdcc161adf34569a5/lib/util.js#L183-L226
[2]: https://github.com/nodejs/node/blob/47804933012841f2dc90626bdcc161adf34569a5/test/parallel/test-util-callbackify.js
[3]: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/7d24857ddb07ab637dfa8c477d13352f8a8206fc/types/node/util.d.ts#L61-L84
[4]: denoland/deno#5366
caspervonb pushed a commit to caspervonb/deno_std that referenced this pull request Jan 24, 2021
This adds [Node's `util.callbackify`][0] to `node/util.ts`.

I lifted most of this from the [original Node source code][1] (and [its
tests][2]). I tried to make minimal modifications to the source.

I made a few arbitrary decisions:

- I was unable to do the function's types elegantly. I made overloads
  for functions that have 0 to 5 (inclusive) arguments, excluding the
  callback. I would love to know a better way to do this. (It seems that
  the folks at DefinitelyTyped [were also stumped][3], though maybe
  their solution is deliberate.)
- There are a few edge cases that cause custom Node errors to be
  produced. Instead of re-implementing those errors completely, I
  created simplified classes. These are mostly correct but are not
  identical to the real Node errors.
- The tests implement a possibly-arcane `TestQueue` class. I originally
  used a lot of inline promises but found it too repetitive.

Closes [denoland/deno#5366][4].

[0]: https://nodejs.org/api/util.html#util_util_callbackify_original
[1]: https://github.com/nodejs/node/blob/47804933012841f2dc90626bdcc161adf34569a5/lib/util.js#L183-L226
[2]: https://github.com/nodejs/node/blob/47804933012841f2dc90626bdcc161adf34569a5/test/parallel/test-util-callbackify.js
[3]: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/7d24857ddb07ab637dfa8c477d13352f8a8206fc/types/node/util.d.ts#L61-L84
[4]: denoland/deno#5366
caspervonb pushed a commit to caspervonb/deno_std that referenced this pull request Jan 24, 2021
This adds [Node's `util.callbackify`][0] to `node/util.ts`.

I lifted most of this from the [original Node source code][1] (and [its
tests][2]). I tried to make minimal modifications to the source.

I made a few arbitrary decisions:

- I was unable to do the function's types elegantly. I made overloads
  for functions that have 0 to 5 (inclusive) arguments, excluding the
  callback. I would love to know a better way to do this. (It seems that
  the folks at DefinitelyTyped [were also stumped][3], though maybe
  their solution is deliberate.)
- There are a few edge cases that cause custom Node errors to be
  produced. Instead of re-implementing those errors completely, I
  created simplified classes. These are mostly correct but are not
  identical to the real Node errors.
- The tests implement a possibly-arcane `TestQueue` class. I originally
  used a lot of inline promises but found it too repetitive.

Closes [denoland/deno#5366][4].

[0]: https://nodejs.org/api/util.html#util_util_callbackify_original
[1]: https://github.com/nodejs/node/blob/47804933012841f2dc90626bdcc161adf34569a5/lib/util.js#L183-L226
[2]: https://github.com/nodejs/node/blob/47804933012841f2dc90626bdcc161adf34569a5/test/parallel/test-util-callbackify.js
[3]: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/7d24857ddb07ab637dfa8c477d13352f8a8206fc/types/node/util.d.ts#L61-L84
[4]: denoland/deno#5366
caspervonb pushed a commit to caspervonb/deno_std that referenced this pull request Jan 31, 2021
This adds [Node's `util.callbackify`][0] to `node/util.ts`.

I lifted most of this from the [original Node source code][1] (and [its
tests][2]). I tried to make minimal modifications to the source.

I made a few arbitrary decisions:

- I was unable to do the function's types elegantly. I made overloads
  for functions that have 0 to 5 (inclusive) arguments, excluding the
  callback. I would love to know a better way to do this. (It seems that
  the folks at DefinitelyTyped [were also stumped][3], though maybe
  their solution is deliberate.)
- There are a few edge cases that cause custom Node errors to be
  produced. Instead of re-implementing those errors completely, I
  created simplified classes. These are mostly correct but are not
  identical to the real Node errors.
- The tests implement a possibly-arcane `TestQueue` class. I originally
  used a lot of inline promises but found it too repetitive.

Closes [denoland/deno#5366][4].

[0]: https://nodejs.org/api/util.html#util_util_callbackify_original
[1]: https://github.com/nodejs/node/blob/47804933012841f2dc90626bdcc161adf34569a5/lib/util.js#L183-L226
[2]: https://github.com/nodejs/node/blob/47804933012841f2dc90626bdcc161adf34569a5/test/parallel/test-util-callbackify.js
[3]: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/7d24857ddb07ab637dfa8c477d13352f8a8206fc/types/node/util.d.ts#L61-L84
[4]: denoland/deno#5366
caspervonb pushed a commit to caspervonb/deno_std that referenced this pull request Jan 31, 2021
This adds [Node's `util.callbackify`][0] to `node/util.ts`.

I lifted most of this from the [original Node source code][1] (and [its
tests][2]). I tried to make minimal modifications to the source.

I made a few arbitrary decisions:

- I was unable to do the function's types elegantly. I made overloads
  for functions that have 0 to 5 (inclusive) arguments, excluding the
  callback. I would love to know a better way to do this. (It seems that
  the folks at DefinitelyTyped [were also stumped][3], though maybe
  their solution is deliberate.)
- There are a few edge cases that cause custom Node errors to be
  produced. Instead of re-implementing those errors completely, I
  created simplified classes. These are mostly correct but are not
  identical to the real Node errors.
- The tests implement a possibly-arcane `TestQueue` class. I originally
  used a lot of inline promises but found it too repetitive.

Closes [denoland/deno#5366][4].

[0]: https://nodejs.org/api/util.html#util_util_callbackify_original
[1]: https://github.com/nodejs/node/blob/47804933012841f2dc90626bdcc161adf34569a5/lib/util.js#L183-L226
[2]: https://github.com/nodejs/node/blob/47804933012841f2dc90626bdcc161adf34569a5/test/parallel/test-util-callbackify.js
[3]: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/7d24857ddb07ab637dfa8c477d13352f8a8206fc/types/node/util.d.ts#L61-L84
[4]: denoland/deno#5366
caspervonb pushed a commit to caspervonb/deno_std that referenced this pull request Jan 31, 2021
This adds [Node's `util.callbackify`][0] to `node/util.ts`.

I lifted most of this from the [original Node source code][1] (and [its
tests][2]). I tried to make minimal modifications to the source.

I made a few arbitrary decisions:

- I was unable to do the function's types elegantly. I made overloads
  for functions that have 0 to 5 (inclusive) arguments, excluding the
  callback. I would love to know a better way to do this. (It seems that
  the folks at DefinitelyTyped [were also stumped][3], though maybe
  their solution is deliberate.)
- There are a few edge cases that cause custom Node errors to be
  produced. Instead of re-implementing those errors completely, I
  created simplified classes. These are mostly correct but are not
  identical to the real Node errors.
- The tests implement a possibly-arcane `TestQueue` class. I originally
  used a lot of inline promises but found it too repetitive.

Closes [denoland/deno#5366][4].

[0]: https://nodejs.org/api/util.html#util_util_callbackify_original
[1]: https://github.com/nodejs/node/blob/47804933012841f2dc90626bdcc161adf34569a5/lib/util.js#L183-L226
[2]: https://github.com/nodejs/node/blob/47804933012841f2dc90626bdcc161adf34569a5/test/parallel/test-util-callbackify.js
[3]: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/7d24857ddb07ab637dfa8c477d13352f8a8206fc/types/node/util.d.ts#L61-L84
[4]: denoland/deno#5366
caspervonb pushed a commit to caspervonb/deno_std that referenced this pull request Jan 31, 2021
This adds [Node's `util.callbackify`][0] to `node/util.ts`.

I lifted most of this from the [original Node source code][1] (and [its
tests][2]). I tried to make minimal modifications to the source.

I made a few arbitrary decisions:

- I was unable to do the function's types elegantly. I made overloads
  for functions that have 0 to 5 (inclusive) arguments, excluding the
  callback. I would love to know a better way to do this. (It seems that
  the folks at DefinitelyTyped [were also stumped][3], though maybe
  their solution is deliberate.)
- There are a few edge cases that cause custom Node errors to be
  produced. Instead of re-implementing those errors completely, I
  created simplified classes. These are mostly correct but are not
  identical to the real Node errors.
- The tests implement a possibly-arcane `TestQueue` class. I originally
  used a lot of inline promises but found it too repetitive.

Closes [denoland/deno#5366][4].

[0]: https://nodejs.org/api/util.html#util_util_callbackify_original
[1]: https://github.com/nodejs/node/blob/47804933012841f2dc90626bdcc161adf34569a5/lib/util.js#L183-L226
[2]: https://github.com/nodejs/node/blob/47804933012841f2dc90626bdcc161adf34569a5/test/parallel/test-util-callbackify.js
[3]: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/7d24857ddb07ab637dfa8c477d13352f8a8206fc/types/node/util.d.ts#L61-L84
[4]: denoland/deno#5366
caspervonb pushed a commit to caspervonb/deno_std that referenced this pull request Feb 1, 2021
This adds [Node's `util.callbackify`][0] to `node/util.ts`.

I lifted most of this from the [original Node source code][1] (and [its
tests][2]). I tried to make minimal modifications to the source.

I made a few arbitrary decisions:

- I was unable to do the function's types elegantly. I made overloads
  for functions that have 0 to 5 (inclusive) arguments, excluding the
  callback. I would love to know a better way to do this. (It seems that
  the folks at DefinitelyTyped [were also stumped][3], though maybe
  their solution is deliberate.)
- There are a few edge cases that cause custom Node errors to be
  produced. Instead of re-implementing those errors completely, I
  created simplified classes. These are mostly correct but are not
  identical to the real Node errors.
- The tests implement a possibly-arcane `TestQueue` class. I originally
  used a lot of inline promises but found it too repetitive.

Closes [denoland/deno#5366][4].

[0]: https://nodejs.org/api/util.html#util_util_callbackify_original
[1]: https://github.com/nodejs/node/blob/47804933012841f2dc90626bdcc161adf34569a5/lib/util.js#L183-L226
[2]: https://github.com/nodejs/node/blob/47804933012841f2dc90626bdcc161adf34569a5/test/parallel/test-util-callbackify.js
[3]: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/7d24857ddb07ab637dfa8c477d13352f8a8206fc/types/node/util.d.ts#L61-L84
[4]: denoland/deno#5366
@EvanHahn EvanHahn deleted the std-node-util-callbackify branch July 2, 2023 23:04
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.

std/node: add util.callbackify
3 participants