-
Notifications
You must be signed in to change notification settings - Fork 30k
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
util.callbackify return value problematic characteristics #26890
Labels
feature request
Issues that request new features to be added to Node.js.
util
Issues and PRs related to the built-in util module.
Comments
BridgeAR
added
util
Issues and PRs related to the built-in util module.
feature request
Issues that request new features to be added to Node.js.
labels
Mar 24, 2019
BridgeAR
added a commit
to BridgeAR/node
that referenced
this issue
Mar 27, 2019
This makes sure the function returned by `util.callbackify()` has a new name that is not identical to the inputs function name. PR-URL: nodejs#26893 Fixes: nodejs#26890 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>
BridgeAR
added a commit
to BridgeAR/node
that referenced
this issue
Mar 27, 2019
Using `util.callbackify()` should not set the prototype for the returned function to the one from the input function. It could cause confusion while debugging otherwise. PR-URL: nodejs#26893 Fixes: nodejs#26890 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>
BethGriggs
pushed a commit
that referenced
this issue
Apr 5, 2019
The returned function from `util.callbackify()` should increase the `length` property by one due to the added callback. PR-URL: #26893 Fixes: #26890 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>
BethGriggs
pushed a commit
that referenced
this issue
Apr 5, 2019
This makes sure the function returned by `util.callbackify()` has a new name that is not identical to the inputs function name. PR-URL: #26893 Fixes: #26890 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>
BethGriggs
pushed a commit
that referenced
this issue
Apr 5, 2019
Using `util.callbackify()` should not set the prototype for the returned function to the one from the input function. It could cause confusion while debugging otherwise. PR-URL: #26893 Fixes: #26890 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>
owenallenaz
added a commit
to simpleviewinc/mongolayer
that referenced
this issue
Jun 4, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
feature request
Issues that request new features to be added to Node.js.
util
Issues and PRs related to the built-in util module.
12.0.0-pre
(d6f6d7f8541327b72667d38777c47
from master) and10.15.1
Linux myhostname 4.4.0-143-generic #169-Ubuntu SMP Thu Feb 7 07:56:38 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
When calling
util.callbackify
on an async function. The returned function has a wronglength
value. It also has the wrong prototype:length
property:Some libraries actually act differently based on the
length
of a function (express, for example: if a handler has a length of four, it's an error handler).Since the returned function has one argument added, it should also add one to the length of the new function. I'm aware that there are several constructs
where the length parameter is wrong (
function
s that are accessing thearguments
, functions defined with...
in the parameter list, etc), but thisbreaks almost all cases and there seems to be no nice workaround to me.
The library I'm dealing with has hooks and it gets determined if they are asyncronous or syncronous based on their
length
(like legacy mocha used to do).prototype
:I understand where this is coming from. There are libraries (like
knex
) that use the builder pattern and have athen
method in their protoype chain for convenience.
(please ignore that knex also provides a callback interface, as this is to make an argument for the current behaviour):
In this case, it might be fine as long as the
query
's prototype hasFunction
in his prototype chain. If not, it is highly confusing to have a function that does not have a normal function's methods.I've looked at the original PR that brought and found a comment that stated it would make not that much difference (note that this is the
promisify
introducing PR, but thecallbackify
one states that it is based upon it).A definitley not
async
Function gets theAsyncFunction.prototype
.Afaik this is just confusing when debugging for now. It does not seem to have any methods on it (until someone sets a
map
method on it).The text was updated successfully, but these errors were encountered: