search

sindresorhus / trash

Move files and directories to the trash
MIT License
2.58k stars 79 forks source link

Trashing a file on Windows deletes it unrecoverably if no recycle bin is enabled #107

Open mifi opened 3 years ago

mifi commented 3 years ago

Similar to #56, but probably has to be solved in a different way for Windows. This was reported by a user in my app losslesscut https://github.com/mifi/lossless-cut/issues/537 When he trashed a file on a network mounted drive (which has no trash), the file just got removed permanently.

sindresorhus commented 3 years ago

What do you think should happen? Throw an error in such case? Add an option for what to do?

mifi commented 3 years ago

IMO it should ideally throw an error when it cannot trash the file, because the reason why I use this library, is so that the user doesn't lose their file immediately. If trash permanently deletes a file, then IMO it did not do its job. But maybe it's not possible to make sure that it never permanently deletes a file.

sindresorhus commented 3 years ago

I agree. It should be a custom Error subclass though, so consumers can easily catch it and decide what to do.

Continued in https://github.com/sindresorhus/recycle-bin/issues/8.

aminya commented 3 years ago

I want to use this package as a fallback for Electron's shell.moveItemToTrash, but it seems Trash removes the files permanently if it fails to trash them.

The electron function is able to detect the errors somehow avoid the deletion.

Edit: They have switched to shell.trashItem recently for the unreleased Electron 13

https://github.com/electron/electron/pull/26723

tomasklaen commented 3 years ago

IMO it should ideally throw an error when it cannot trash the file, because the reason why I use this library, is so that the user doesn't lose their file immediately.

I see the reasoning here, but it conflicts with a use case where Windows user can configure their Recycle bin to delete files immediately. In this scenario deleting files instead of moving them to trash is what the user wants, and throwing errors here would be wrong.

So unless you can check that file is being deleted from a drive that doesn't have immediate deletion enabled, you shouldn't throw errors.