search

filerjs / filer

Node-like file system for browsers
BSD 2-Clause "Simplified" License
613 stars 153 forks source link

BUG: fs.readdir fails when passing options #775

Closed bcheidemann closed 3 years ago

bcheidemann commented 3 years ago

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 commented 3 years ago

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 https://github.com/filerjs/filer/blob/75f2a70c7b8111987f0df6c9105c1b6c65bef3c4/src/shell/shell.js#L205-L280.

bcheidemann commented 3 years ago

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

https://github.com/filerjs/filer/blob/75f2a70c7b8111987f0df6c9105c1b6c65bef3c4/src/shell/shell.js#L205-L280

.

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 commented 3 years ago

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

bcheidemann commented 3 years ago

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 https://github.com/filerjs/filer/blob/75f2a70c7b8111987f0df6c9105c1b6c65bef3c4/src/shell/shell.js#L205-L280

.

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