search

jprichardson / node-fs-extra

Node.js: extra methods for the fs object like copy(), remove(), mkdirs()
MIT License
9.43k stars 775 forks source link

Feature request: isEmptyDir() #1037

Open btea opened 5 months ago

btea commented 5 months ago

Sometimes we encounter situations where we need to determine whether a directory is empty before deleting or performing other operations. Therefore, I think it might be convenient if an API can be provided.

fs.isEmptyDir(dir[,callback])
RyanZim commented 5 months ago

This is basically a duplicate of https://github.com/jprichardson/node-fs-extra/issues/420; though I'm open to reconsidering the decision there.

Can you elaborate more on the use-case here?

btea commented 5 months ago

This is basically a duplicate of #420; though I'm open to reconsidering the decision there.

Oh, sorry, I didn't notice this.

Can you elaborate more on the use-case here?

For example, in pnpm, there is a pnpm patch-remove command. After removing a certain patch file, you need to determine whether the directory where the patch file is stored is empty to decide whether to remove the corresponding folder.

There is also scaffolding that when initializing a project, it will also judge whether it is empty to decide whether to overwrite the content and continue or cancel. And I think this method can also be reused internally in emptyDir.

RyanZim commented 5 months ago

I'm especially hesitant to add it as it's trivial to implement wherever it's needed; the code is literally:

!(await fs.readdir(dir)).length

Is it worth creating a method just for this?

RyanZim commented 5 months ago

Also, in the case of something like emptyDir, you wouldn't actually want to use this, as you want the full output from fs.readdir anyhow.

btea commented 5 months ago

I'm especially hesitant to add it as it's trivial to implement wherever it's needed; the code is literally:

!(await fs.readdir(dir)).length

Yes, the code implementation seems relatively simple.

Is it worth creating a method just for this?

It is hoped that providing this wrapper function is just a suggestion. It is difficult for me to give a strong support for whether it is worth adding. The scenario I am currently encountering is the example I provided above.

RyanZim commented 5 months ago

cc @JPeer264 @manidlou @jprichardson for thoughts

SukkaW commented 1 month ago

@RyanZim I have some thoughts.

fs.readdir can be extremely slow if a folder contains many items (that's why https://github.com/jprichardson/node-fs-extra/pull/1028 is made, which replaces readdir w/ opendir).

fs.opendir is more performant (since it implements stream API), but it can't be used in a one-liner like !(await fs.readdir(dir)).length. Thus a util isEmptyDir using fs.opendir under the hood is trivial for me.

RyanZim commented 1 month ago

@SukkaW That's an interesting point; in that case I'd be open to a PR adding it.

SukkaW commented 1 month ago

@SukkaW That's an interesting point; in that case I'd be open to a PR adding it.

I can create a PR adding it!