Skip to content
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.

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

FS API changes #74

Open
7 of 11 tasks
cookiengineer opened this issue Apr 17, 2017 · 3 comments
Open
7 of 11 tasks

FS API changes #74

cookiengineer opened this issue Apr 17, 2017 · 3 comments

Comments

@cookiengineer
Copy link
Contributor

cookiengineer commented Apr 17, 2017

I was trying to implement mkDir() today, and I figured out that most of the File API is somehow very hard to understand, especially the static methods that can be used like File.readSync(path).

After discussing some things with @paraboul I'd suggest that we need to move some methods from File to the FS object, so that static methods are in one place and not confusing anymore:

Changes to File

  • Remove File.prototype.rm()
  • Remove File.prototype.rmrf()
  • Remove File.prototype.isDir()
  • Remove File.prototype.listFiles() because fs.readDir() already has identical featureset

Changes to fs

  • Create fs.remove(path, (err) => {})
  • Create (Boolean) fs.removeSync(path)
  • Create fs.createDir(path, (err) => {})
  • Create (Boolean) fs.createDirSync(path, options)
  • Create (Array) fs.readDirSync(path)
  • Create (Boolean) fs.isDir(path)
  • Create (Boolean) fs.isFile(path)

So here's some code how it would look like:

const _fs = global.fs;
const _File = global.File;

if (_fs.isDir('/tmp/whatever') === false) {
    _fs.createDir('/tmp/whatever', err => !err && console.log('created directory'));

    // alternative api
    let result = _fs.createDirSync('/tmp/whatever');
}

if (_fs.isFile('/tmp/whatever/foo.txt') === true) {
    _fs.remove('/tmp/whatever/foo.txt', err => !err && console.log('removed file'));

    // alternative api
    let result = _fs.removeSync('/tmp/whatever/foo.txt');

}

PS: I would love to implement this as my first pull request, as it seems that static methods are easier to implement and the relevant method signatures are already in the codebase.

@cookiengineer
Copy link
Contributor Author

ping @verpeteren and @efyx on their opinion.

@cookiengineer
Copy link
Contributor Author

Started work on above APIs on this branch: https://github.com/cookiengineer/Nidium/tree/feature/fs

@cookiengineer
Copy link
Contributor Author

cookiengineer commented Apr 18, 2017

Can somebody help me understand how the tasks and callbacks work or would be implemented, e.g. in the case for fs.readDir() and fs.readDirSync()?

I currently have troubles making sense of fs.readDir() because it's firing the callback multiple times, for every single file item. Is that the wanted behaviour?

I would guess this is like totally unusable as it will eventually lead to people having to use a cache array anyways... and therefore would just harden the usage of the API.

Implementation-wise remove, readDir, readDirSync, createDir and createDirSync are missing. Other methods are implemented, but undocumented and yet have no unit tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant