diff --git a/support/isFunctionLengthConfigurable.js b/support/isFunctionLengthConfigurable.js new file mode 100644 index 0000000..9c8d771 --- /dev/null +++ b/support/isFunctionLengthConfigurable.js @@ -0,0 +1,4 @@ + +module.exports = function isFunctionLengthConfigurable(arg) { + return Object.getOwnPropertyDescriptor(function () { }, 'length').configurable; +} \ No newline at end of file diff --git a/support/nodeJSVersion.js b/support/nodeJSVersion.js new file mode 100644 index 0000000..b00ce7a --- /dev/null +++ b/support/nodeJSVersion.js @@ -0,0 +1,8 @@ + +module.exports = function nodeJSVersion(arg) { + + if (!process || !process.version || !process.version.match(/^v(\d+)\.(\d+)/)) { + return false; + } + return parseInt(process.version.split('.')[0].slice(1), 10); +} diff --git a/test/browser/callbackify.js b/test/browser/callbackify.js index ea05089..b6f32cc 100644 --- a/test/browser/callbackify.js +++ b/test/browser/callbackify.js @@ -3,6 +3,8 @@ var test = require('tape'); var callbackify = require('../../').callbackify; +var isFunctionLengthConfigurable = require('../../support/isFunctionLengthConfigurable'); + if (typeof Promise === 'undefined') { console.log('no global Promise found, skipping callbackify tests'); return; @@ -167,3 +169,23 @@ test('util.callbackify non-function inputs throw', function (t) { }); t.end(); }); + +test('util.callbackify resulting function should have one more argument', function (t) { + + if (!isFunctionLengthConfigurable()) { + console.log("skipping this test as function.length is not configurable in the current javascript engine"); + return; + } + // Test that resulting function should have one more argument + [ + function() { }, + function(a) { }, + function(a, b) { } + ].forEach(function (fct) { + + var callbackified = callbackify(fct); + t.strictEqual(callbackified.length, fct.length + 1, "callbackified function should have one more argument"); + }); + t.end(); +}); + diff --git a/test/node/callbackify.js b/test/node/callbackify.js index 8da100a..c20a3b1 100644 --- a/test/node/callbackify.js +++ b/test/node/callbackify.js @@ -7,6 +7,39 @@ var common = require('./common'); var assert = require('assert'); var callbackify = require('../../').callbackify; var execFile = require('child_process').execFile; +var nodeJSVersion = require('../../support/nodeJSVersion'); +var isFunctionLengthConfigurable = require("../../support/isFunctionLengthConfigurable"); + +(function callbackify_resulting_function_should_have_one_more_argument() { + + if (!isFunctionLengthConfigurable()) { + console.log("skipping this test as function.length is not configurable in the current javascript engine"); + return; + } + var nodeVersion = nodeJSVersion(); + + console.log("Testing callbackify resulting function should have one more argument") + var original_callbackify = require('util').callbackify; + // Test that resulting function should have one more argument + [ + function () { }, + function (a) { }, + function (a, b) { } + ].forEach(function (fct) { + + var node_callbackified = original_callbackify(fct); + var browser_callbackified = callbackify(fct); + + if (nodeVersion >= 12 && node_callbackified.length !== fct.length + 1) { + // this behavior is only true with node 12 and above, where the bug was fixed + throw new Error("callbackified function should have one more argument"); + } + if (browser_callbackified.length !== node_callbackified.length) { + console.log("browser_callbackified=", browser_callbackified.length, "node_callbackified=", node_callbackified.length) + throw new Error("callbackified function should have one more argument, like in node"); + } + }); +})(); if (typeof Promise === 'undefined') { console.log('no global Promise found, skipping callbackify tests'); diff --git a/util.js b/util.js index 6eea657..8d5442a 100644 --- a/util.js +++ b/util.js @@ -703,13 +703,21 @@ function callbackify(original) { // In true node style we process the callback on `nextTick` with all the // implications (stack, `uncaughtException`, `async_hooks`) original.apply(this, args) - .then(function(ret) { process.nextTick(cb.bind(null, null, ret)) }, - function(rej) { process.nextTick(callbackifyOnRejected.bind(null, rej, cb)) }); + .then(function (ret) { process.nextTick(cb.bind(null, null, ret)) }, + function (rej) { process.nextTick(callbackifyOnRejected.bind(null, rej, cb)) }); } Object.setPrototypeOf(callbackified, Object.getPrototypeOf(original)); - Object.defineProperties(callbackified, - getOwnPropertyDescriptors(original)); + const desc = getOwnPropertyDescriptors(original); + var isFunctionLengthConfigurable = Object.getOwnPropertyDescriptor(callbackified, 'length').configurable; + + // versions of NodeJS <12.0 have a bug where the callback's `length` is not adjusted as in recent NodeJS version + // even though functionLength is configurable. + if (isFunctionLengthConfigurable) { + desc.length.value += 1; + } + + Object.defineProperties(callbackified, desc); return callbackified; } exports.callbackify = callbackify;