Skip to content

lib: consistent this in callbacks #14645

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.

Sign up for GitHub

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

Closed
wants to merge 5 commits into from
+75 −20
Closed

lib: consistent this in callbacks #14645

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b3166e3
test: refactor test-fs-stat
Trott Aug 6, 2017
9d6365a
fs: invoke callbacks with undefined context
Trott Aug 6, 2017
23918c4
test: check `this` value for `nextTick()`
Trott Aug 6, 2017
aa07e87
process: make `this` value consistent
Trott Aug 6, 2017
64e1f00
benchmark: cover more nextTick() code
Trott Aug 6, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion benchmark/process/next-tick-breadth-args.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,17 @@ function main(conf) {
if (n === N)
bench.end(n / 1e6);
}
function cb4(arg1, arg2, arg3, arg4) {
n++;
if (n === N)
bench.end(n / 1e6);
}

bench.start();
for (var i = 0; i < N; i++) {
if (i % 3 === 0)
if (i % 4 === 0)
process.nextTick(cb4, 3.14, 1024, true, false);
else if (i % 3 === 0)
process.nextTick(cb3, 512, true, null);
else if (i % 2 === 0)
process.nextTick(cb2, false, 5.1);
Expand Down
25 changes: 22 additions & 3 deletions benchmark/process/next-tick-depth-args.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,24 @@ process.maxTickDepth = Infinity;
function main(conf) {
var n = +conf.millions * 1e6;

function cb4(arg1, arg2, arg3, arg4) {
if (--n) {
if (n % 4 === 0)
process.nextTick(cb4, 3.14, 1024, true, false);
else if (n % 3 === 0)
process.nextTick(cb3, 512, true, null);
else if (n % 2 === 0)
process.nextTick(cb2, false, 5.1);
else
process.nextTick(cb1, 0);
} else
bench.end(+conf.millions);
}
function cb3(arg1, arg2, arg3) {
if (--n) {
if (n % 3 === 0)
if (n % 4 === 0)
process.nextTick(cb4, 3.14, 1024, true, false);
else if (n % 3 === 0)
process.nextTick(cb3, 512, true, null);
else if (n % 2 === 0)
process.nextTick(cb2, false, 5.1);
Expand All @@ -23,7 +38,9 @@ function main(conf) {
}
function cb2(arg1, arg2) {
if (--n) {
if (n % 3 === 0)
if (n % 4 === 0)
process.nextTick(cb4, 3.14, 1024, true, false);
else if (n % 3 === 0)
process.nextTick(cb3, 512, true, null);
else if (n % 2 === 0)
process.nextTick(cb2, false, 5.1);
Expand All @@ -34,7 +51,9 @@ function main(conf) {
}
function cb1(arg1) {
if (--n) {
if (n % 3 === 0)
if (n % 4 === 0)
process.nextTick(cb4, 3.14, 1024, true, false);
else if (n % 3 === 0)
process.nextTick(cb3, 512, true, null);
else if (n % 2 === 0)
process.nextTick(cb2, false, 5.1);
Expand Down
2 changes: 1 addition & 1 deletion lib/fs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ function makeCallback(cb) {
}

return function() {
return cb.apply(null, arguments);
return cb.apply(undefined, arguments);
};
}

Expand Down
2 changes: 1 addition & 1 deletion lib/internal/process/next_tick.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ function setupNextTick() {
callback(args[0], args[1], args[2]);
break;
default:
callback.apply(null, args);
callback(...args);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-fs-mkdtemp.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ assert(common.fileExists(utf8));
function handler(err, folder) {
assert.ifError(err);
assert(common.fileExists(folder));
assert.strictEqual(this, null);
assert.strictEqual(this, undefined);
}

fs.mkdtemp(path.join(common.tmpDir, 'bar.'), common.mustCall(handler));
Expand Down
39 changes: 26 additions & 13 deletions test/parallel/test-fs-stat.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,19 @@
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
// USE OR OTHER DEALINGS IN THE SOFTWARE.

/* eslint-disable strict */
'use strict';
const common = require('../common');

const assert = require('assert');
const fs = require('fs');

fs.stat('.', common.mustCall(function(err, stats) {
assert.ifError(err);
assert.ok(stats.mtime instanceof Date);
assert.strictEqual(this, global);
// Confirm that we are not running in the context of the internal binding
// layer.
// Ref: https://github.com/nodejs/node/commit/463d6bac8b349acc462d345a6e298a76f7d06fb1
assert.strictEqual(this, undefined);
}));

fs.stat('.', common.mustCall(function(err, stats) {
Expand All @@ -38,22 +42,31 @@ fs.stat('.', common.mustCall(function(err, stats) {
fs.lstat('.', common.mustCall(function(err, stats) {
assert.ifError(err);
assert.ok(stats.mtime instanceof Date);
assert.strictEqual(this, global);
// Confirm that we are not running in the context of the internal binding
// layer.
// Ref: https://github.com/nodejs/node/commit/463d6bac8b349acc462d345a6e298a76f7d06fb1
assert.strictEqual(this, undefined);
}));

// fstat
fs.open('.', 'r', undefined, common.mustCall(function(err, fd) {
assert.ok(!err);
assert.ifError(err);
assert.ok(fd);

fs.fstat(fd, common.mustCall(function(err, stats) {
assert.ifError(err);
assert.ok(stats.mtime instanceof Date);
fs.close(fd, assert.ifError);
assert.strictEqual(this, global);
// Confirm that we are not running in the context of the internal binding
// layer.
// Ref: https://github.com/nodejs/node/commit/463d6bac8b349acc462d345a6e298a76f7d06fb1
assert.strictEqual(this, undefined);
}));

assert.strictEqual(this, global);
// Confirm that we are not running in the context of the internal binding
// layer.
// Ref: https://github.com/nodejs/node/commit/463d6bac8b349acc462d345a6e298a76f7d06fb1
assert.strictEqual(this, undefined);
}));

// fstatSync
Expand All @@ -72,13 +85,13 @@ fs.open('.', 'r', undefined, common.mustCall(function(err, fd) {

fs.stat(__filename, common.mustCall(function(err, s) {
assert.ifError(err);
assert.strictEqual(false, s.isDirectory());
assert.strictEqual(true, s.isFile());
assert.strictEqual(false, s.isSocket());
assert.strictEqual(false, s.isBlockDevice());
assert.strictEqual(false, s.isCharacterDevice());
assert.strictEqual(false, s.isFIFO());
assert.strictEqual(false, s.isSymbolicLink());
assert.strictEqual(s.isDirectory(), false);
assert.strictEqual(s.isFile(), true);
assert.strictEqual(s.isSocket(), false);
assert.strictEqual(s.isBlockDevice(), false);
assert.strictEqual(s.isCharacterDevice(), false);
assert.strictEqual(s.isFIFO(), false);
assert.strictEqual(s.isSymbolicLink(), false);
const keys = [
'dev', 'mode', 'nlink', 'uid',
'gid', 'rdev', 'ino', 'size',
Expand Down
16 changes: 16 additions & 0 deletions test/parallel/test-next-tick.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

'use strict';
const common = require('../common');

const assert = require('assert');

process.nextTick(common.mustCall(function() {
Expand All @@ -40,8 +41,23 @@ const obj = {};
process.nextTick(function(a, b) {
assert.strictEqual(a, 42);
assert.strictEqual(b, obj);
assert.strictEqual(this, undefined);
}, 42, obj);

process.nextTick((a, b) => {
assert.strictEqual(a, 42);
assert.strictEqual(b, obj);
assert.deepStrictEqual(this, {});
}, 42, obj);

process.nextTick(function() {
assert.strictEqual(this, undefined);
}, 1, 2, 3, 4);

process.nextTick(() => {
assert.deepStrictEqual(this, {});
}, 1, 2, 3, 4);

process.on('exit', function() {
process.nextTick(common.mustNotCall());
});