BUG: fs.readdir fails when passing options

[bcheidemann]

The following code:

fs.readdir('/somedir', {/* options */}, callback);

Throws the error:
Error: Uncaught TypeError: callback is not a function (http://localhost:1234/tests.e31bb0bc.js:17740) at commonjsGlobal.onerror (mocha.947ba722.js:32655:12)

According to the node documentation, fs.readdir and fs.promises.readdir should optionally accept an options object.

[humphd]

Nice find.

We sort of have code to do this already in the shell, but it could be ported into the implementation for when { recursive: true } is present, see

filer/src/shell/shell.js

Lines 205 to 280 in 75f2a70

	/**
	* Get the listing of a directory, returning an array of
	* file entries in the following form:
	*
	* {
	* path: <String> the basename of the directory entry
	* links: <Number> the number of links to the entry
	* size: <Number> the size in bytes of the entry
	* modified: <Number> the last modified date/time
	* type: <String> the type of the entry
	* contents: <Array> an optional array of child entries
	* }
	*
	* By default ls() gives a shallow listing. If you want
	* to follow directories as they are encountered, use
	* the `recursive=true` option.
	*/
	Shell.prototype.ls = function(dir, options, callback) {
	var sh = this;
	var fs = sh.fs;
	if(typeof options === 'function') {
	callback = options;
	options = {};
	}
	options = options || {};
	callback = callback || function(){};
	if(!dir) {
	callback(new Errors.EINVAL('Missing dir argument'));
	return;
	}
	function list(path, callback) {
	var pathname = Path.resolve(sh.pwd(), path);
	var result = [];
	fs.readdir(pathname, function(error, entries) {
	if(error) {
	callback(error);
	return;
	}
	function getDirEntry(name, callback) {
	name = Path.join(pathname, name);
	fs.stat(name, function(error, stats) {
	if(error) {
	callback(error);
	return;
	}
	var entry = stats;
	if(options.recursive && stats.type === 'DIRECTORY') {
	list(Path.join(pathname, entry.name), function(error, items) {
	if(error) {
	callback(error);
	return;
	}
	entry.contents = items;
	result.push(entry);
	callback();
	});
	} else {
	result.push(entry);
	callback();
	}
	});
	}
	async.eachSeries(entries, getDirEntry, function(error) {
	callback(error, result);
	});
	});
	}
	list(dir, callback);
	};

.

[bcheidemann]

Nice find.

We sort of have code to do this already in the shell, but it could be ported into the implementation for when { recursive: true } is present, see

filer/src/shell/shell.js

Lines 205 to 280 in 75f2a70

	/**
	* Get the listing of a directory, returning an array of
	* file entries in the following form:
	*
	* {
	* path: <String> the basename of the directory entry
	* links: <Number> the number of links to the entry
	* size: <Number> the size in bytes of the entry
	* modified: <Number> the last modified date/time
	* type: <String> the type of the entry
	* contents: <Array> an optional array of child entries
	* }
	*
	* By default ls() gives a shallow listing. If you want
	* to follow directories as they are encountered, use
	* the `recursive=true` option.
	*/
	Shell.prototype.ls = function(dir, options, callback) {
	var sh = this;
	var fs = sh.fs;
	if(typeof options === 'function') {
	callback = options;
	options = {};
	}
	options = options || {};
	callback = callback || function(){};
	if(!dir) {
	callback(new Errors.EINVAL('Missing dir argument'));
	return;
	}
	function list(path, callback) {
	var pathname = Path.resolve(sh.pwd(), path);
	var result = [];
	fs.readdir(pathname, function(error, entries) {
	if(error) {
	callback(error);
	return;
	}
	function getDirEntry(name, callback) {
	name = Path.join(pathname, name);
	fs.stat(name, function(error, stats) {
	if(error) {
	callback(error);
	return;
	}
	var entry = stats;
	if(options.recursive && stats.type === 'DIRECTORY') {
	list(Path.join(pathname, entry.name), function(error, items) {
	if(error) {
	callback(error);
	return;
	}
	entry.contents = items;
	result.push(entry);
	callback();
	});
	} else {
	result.push(entry);
	callback();
	}
	});
	}
	async.eachSeries(entries, getDirEntry, function(error) {
	callback(error, result);
	});
	});
	}
	list(dir, callback);
	};

.

Happy to update PR #778 to include the required changes but may not have time until next weekend. Looks like node supports encoding and withFileTypes options but not recursive for readdir. Do you want to implement it with support for recursive anyway?

[humphd]

Ah, you're right. If node doesn't, let's not do it either, and leave ls for this case.

[bcheidemann]

Nice find.
We sort of have code to do this already in the shell, but it could be ported into the implementation for when { recursive: true } is present, see

filer/src/shell/shell.js

Lines 205 to 280 in 75f2a70

	/**
	* Get the listing of a directory, returning an array of
	* file entries in the following form:
	*
	* {
	* path: <String> the basename of the directory entry
	* links: <Number> the number of links to the entry
	* size: <Number> the size in bytes of the entry
	* modified: <Number> the last modified date/time
	* type: <String> the type of the entry
	* contents: <Array> an optional array of child entries
	* }
	*
	* By default ls() gives a shallow listing. If you want
	* to follow directories as they are encountered, use
	* the `recursive=true` option.
	*/
	Shell.prototype.ls = function(dir, options, callback) {
	var sh = this;
	var fs = sh.fs;
	if(typeof options === 'function') {
	callback = options;
	options = {};
	}
	options = options || {};
	callback = callback || function(){};
	if(!dir) {
	callback(new Errors.EINVAL('Missing dir argument'));
	return;
	}
	function list(path, callback) {
	var pathname = Path.resolve(sh.pwd(), path);
	var result = [];
	fs.readdir(pathname, function(error, entries) {
	if(error) {
	callback(error);
	return;
	}
	function getDirEntry(name, callback) {
	name = Path.join(pathname, name);
	fs.stat(name, function(error, stats) {
	if(error) {
	callback(error);
	return;
	}
	var entry = stats;
	if(options.recursive && stats.type === 'DIRECTORY') {
	list(Path.join(pathname, entry.name), function(error, items) {
	if(error) {
	callback(error);
	return;
	}
	entry.contents = items;
	result.push(entry);
	callback();
	});
	} else {
	result.push(entry);
	callback();
	}
	});
	}
	async.eachSeries(entries, getDirEntry, function(error) {
	callback(error, result);
	});
	});
	}
	list(dir, callback);
	};

.

Happy to update PR #778 to include the required changes but may not have time until next weekend. Looks like node supports encoding and withFileTypes options but not recursive for readdir. Do you want to implement it with support for recursive anyway?

@humphd added a skip as you suggested here. I'll make a new PR for the fixes if you want to merge #778