Skip to content

Commit

Permalink
fs: make fs.read params optional
Browse files Browse the repository at this point in the history
This makes all the parameters of the `fs.read` function, except
for `fd` and the callback(when not using as a promise) optional.

They will default to sensible defaults.

Fixes: #31237

PR-URL: #31402
Reviewed-By: Robert Nagy <ronagy@icloud.com>
  • Loading branch information
lholmquist authored and targos committed Apr 28, 2020
1 parent 40c5d58 commit 16a913f
Show file tree
Hide file tree
Showing 4 changed files with 115 additions and 0 deletions.
34 changes: 34 additions & 0 deletions doc/api/fs.md
Original file line number Diff line number Diff line change
Expand Up @@ -2775,6 +2775,29 @@ The callback is given the three arguments, `(err, bytesRead, buffer)`.
If this method is invoked as its [`util.promisify()`][]ed version, it returns
a `Promise` for an `Object` with `bytesRead` and `buffer` properties.

## `fs.read(fd, [options,] callback)`
<!-- YAML
added: REPLACEME
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/31402
description: Options object can be passed in
to make Buffer, offset, length and position optional
-->
* `fd` {integer}
* `options` {Object}
* `buffer` {Buffer|TypedArray|DataView} **Default:** `Buffer.alloc(16384)`
* `offset` {integer} **Default:** `0`
* `length` {integer} **Default:** `buffer.length`
* `position` {integer} **Default:** `null`
* `callback` {Function}
* `err` {Error}
* `bytesRead` {integer}
* `buffer` {Buffer}

Similar to the above `fs.read` function, this version takes an optional `options` object.
If no `options` object is specified, it will default with the above values.

## `fs.readdir(path[, options], callback)`
<!-- YAML
added: v0.1.8
Expand Down Expand Up @@ -4365,6 +4388,17 @@ Following successful read, the `Promise` is resolved with an object with a
`bytesRead` property specifying the number of bytes read, and a `buffer`
property that is a reference to the passed in `buffer` argument.

#### `filehandle.read(options)`
<!-- YAML
added: REPLACEME
-->
* `options` {Object}
* `buffer` {Buffer|Uint8Array} **Default:** `Buffer.alloc(16384)`
* `offset` {integer} **Default:** `0`
* `length` {integer} **Default:** `buffer.length`
* `position` {integer} **Default:** `null`
* Returns: {Promise}

#### `filehandle.readFile(options)`
<!-- YAML
added: v10.0.0
Expand Down
26 changes: 26 additions & 0 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -459,8 +459,34 @@ function openSync(path, flags, mode) {
return result;
}

// usage:
// fs.read(fd, buffer, offset, length, position, callback);
// OR
// fs.read(fd, {}, callback)
function read(fd, buffer, offset, length, position, callback) {
validateInt32(fd, 'fd', 0);

if (arguments.length <= 3) {
// Assume fs.read(fd, options, callback)
let options = {};
if (arguments.length < 3) {
// This is fs.read(fd, callback)
// buffer will be the callback
callback = buffer;
} else {
// This is fs.read(fd, {}, callback)
// buffer will be the options object
// offset is the callback
options = buffer;
callback = offset;
}

buffer = options.buffer || Buffer.alloc(16384);
offset = options.offset || 0;
length = options.length || buffer.length;
position = options.position;
}

validateBuffer(buffer);
callback = maybeCallback(callback);

Expand Down
36 changes: 36 additions & 0 deletions test/parallel/test-fs-read-optional-params.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
'use strict';

const common = require('../common');
const fixtures = require('../common/fixtures');
const fs = require('fs');
const assert = require('assert');
const filepath = fixtures.path('x.txt');
const fd = fs.openSync(filepath, 'r');

const expected = Buffer.from('xyz\n');
const defaultBufferAsync = Buffer.alloc(16384);
const bufferAsOption = Buffer.allocUnsafe(expected.length);

// Test passing in an empty options object
fs.read(fd, { position: 0 }, common.mustCall((err, bytesRead, buffer) => {
assert.strictEqual(bytesRead, expected.length);
assert.deepStrictEqual(defaultBufferAsync.length, buffer.length);
}));

// Test not passing in any options object
fs.read(fd, common.mustCall((err, bytesRead, buffer) => {
assert.strictEqual(bytesRead, expected.length);
assert.deepStrictEqual(defaultBufferAsync.length, buffer.length);
}));

// Test passing in options
fs.read(fd, {
buffer: bufferAsOption,
offset: 0,
lenght: bufferAsOption.length,
position: 0
},
common.mustCall((err, bytesRead, buffer) => {
assert.strictEqual(bytesRead, expected.length);
assert.deepStrictEqual(bufferAsOption.length, buffer.length);
}));
19 changes: 19 additions & 0 deletions test/parallel/test-fs-read-promises-optional-params.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
'use strict';

const common = require('../common');
const fixtures = require('../common/fixtures');
const fs = require('fs');
const read = require('util').promisify(fs.read);
const assert = require('assert');
const filepath = fixtures.path('x.txt');
const fd = fs.openSync(filepath, 'r');

const expected = Buffer.from('xyz\n');
const defaultBufferAsync = Buffer.alloc(16384);

read(fd, {})
.then(function({ bytesRead, buffer }) {
assert.strictEqual(bytesRead, expected.length);
assert.deepStrictEqual(defaultBufferAsync.length, buffer.length);
})
.then(common.mustCall());

0 comments on commit 16a913f

Please sign in to comment.