fsx.delete dir scares me

[papaben]

First of all -- I love this library, thank you!

My feedback is about the risks versus the ease of delete(). I have attended more than one postmortem caused by someone accidentally deleting a directory caused by a bug further up the stack. e.g.

// Oops, a bug in this method returns the directory name concatenated with an empty string
const fileName = getFileName()

// Accidentally delete every file in the directory =( 
fsx.delete(fileName)

Having a flag or separate method is an extra layer of defense against unintended behavior like that.

[nzakas]

Thanks for the feedback. This is something I considered too, but decided I'd go aggressive out of the gate and wait for feedback to come in.

What would you rather see instead? Two different methods, one that deletes files and one that deletes directories?

[papaben]

I'm actually not sure what is the most idiomatic. I thought some more about use cases. Most of the file interaction that I do is typically with temporary files, which don't need to be deleted by me explicitly. An example of a time that I'd be removing non-temporary files might be if I'm doing something like a build manager. Whatever the case, I'd always know for certain when I'm deleting a file versus the entire directory.

Option 1: fsx.deleteFile(path) and fsx.deleteDir(path)

Pros: it's explicit, no method parameters to remember.
Cons: Bloats module signature

Option 2: fsx.delete(path, isDir=false)

Pros: Single method
Cons: Default arguments bloat the method signature =)

Option 3: fsx.delete.file(path) and fsx.delete.dir(path)

Pros: scopes any sort of "delete" activity to a sub-module
Cons: Feels like overkill

Conclusion

Perhaps making a decision on this may require taking a bigger step back to consider how you are going to approach overloading methods on the entire module.

[nzakas]

In general, I'm avoiding adding "options" parameters to methods. I think that's part of what makes the Node.js fs API lousy to work with, so option 2 is out. Option 3 definitely feels like overkill.

I'm kind of leaning towards fsx.delete() for files and fsx.deleteDirectory() for directories (this would match fsx.createDirectory() and fsx.isDirectory(), as I'm avoiding abbreviations in the method names. I'm guessing that deleting files will be more common than deleting directories, so I don't mind giving that a shorter method name.

[papaben]

Thanks for the nice discussion.

[airamrguez]

IMHO, I'd rather have the current delete function.

This also sticks with de same behavior of creating directories if needed with the write function.

If you want to delete a file and you are worried about deleting the wrong thing you should use fsx.isFile before.

Having a small API will also make things simple.

[nzakas]

The argument here is that most people will forget to check if a path represents a file before calling delete(), which basically makes this a footgun similar to fs.stat() where people will forget to wrap it in a try-catch.