Add options to fse.remove*() ?

[RyanZim]

Opening a new issue to continue discussions from https://github.com/jprichardson/node-fs-extra/issues/246 & https://github.com/jprichardson/node-fs-extra/pull/785 on a fresh thread. CC: @CxRes @kutyepov

There have been various proposals to add an option that would make fse.remove error on nonexistent paths. Given that Node's new fs.rm method supports this (https://github.com/jprichardson/node-fs-extra/issues/806), I'm open to the idea. However, I would appreciate input from the other maintainers here.

Another remaining question is what this option should be named. force would follow fs.rm, but it would be a default-true option, which I kind of dislike. shoutMissing was proposed in https://github.com/jprichardson/node-fs-extra/pull/785, but I'm not strongly for or against that name. Naming stuff is hard, ideas wanted.

[manidlou]

Yeah since Node has that option, we should add that too. I also agree with you that we should use fs.rm wherever is possible.

Naming stuff is hard

:smile: agreed! I am not sure about the name tho! Maybe errorIfNotExists?!

[CxRes]

Should we not align ourselves with fs.rm, effectively back porting it to earlier editions of node? Unless there is a strong reason for introducing a different behaviour?

[RyanZim]

Our default behavior cannot change, and our default behavior is the exact opposite of fs.rm. fse.remove currently behaves like fs.rm with recursive, force.

[CxRes]

From what I recall, this default behavior was chosen because of the rmdir -r choice made by nodejs. In the light of present changes to node's api, I would think fse should ideally not continue with this kitchen sink choice. On the other hand, I understand that we do not want to break user-land. Perhaps, a new namespace like fse.delete?

[RyanZim]

From what I recall, this default behavior was chosen because of the rmdir -r choice made by nodejs.

No, fse.remove has worked the way it does since the beginning of fs-extra, which far predates Node having any kind of recursive function. fse.remove cannot change its behavior.

The way I see it, we have 3 options:

1. Add options to fse.remove to make it function more like fs.rm, but keep default behavior.
   • This has the advantage of being able to reuse existing code.
2. Add a new method that's like fs.rm (i.e. your fse.delete proposal).
   • This requires duplicating code, and creates a method that will have to be removed in the future.
3. Polyfill fs.rm for older versions of Node.
   • AFAIK, we've never done polyfilling of Node methods. This requires precision to match Node's behavior, but does have a strong argument for being the least disruptive to users of fs-extra.

[kutyepov]

What if fse.remove is changed to error out when a file is not present by default? This will align it with node's rm function and won't introduce additional overhead in maintaining the library. I understand that this will be a breaking change, but that could be released as a major version of the library.

[RyanZim]

fse.remove has existed with its current behavior since the inception of this library AFAIK; changing default behavior is non-negotiable.

[RyanZim]

fs.rm is supported in all maintained LTS versions of Node, so closing this out. If you want fs.rm behavior, use fs.rm.

[RyanZim]

As per https://github.com/jprichardson/node-fs-extra/issues/968#issuecomment-1286003266; fse.remove will become a thin wrapper around fs.rm.