Migrate from rimraf to NodeJS fs

[confused-Techie]

In this repo we have the module dependency rimraf which is used in a singular location to bring unix like file deletions into the copy and move commands.

With such little usage, it seemed easy enough to migrate this to the builtin NodeJS fs module's rm method.

After migrating that then means we are able to fully go ahead and remove the rimraf dependency from this repository completely.

As for any concerns about the change, the options provided to fs.rm, according to the docs, will mimic Unix rm -rf functionality. Which is quite literally the repo description of rimraf.

[DeeDeeG]

Removing rimraf is great!

I'm neutral on promises-based fs vs callbacks-based fs. Neither has massively cleaner/more-readable syntax here, IMO. But this is fine as it is. It took me a moment to spot the .then() and realize how the code flow was working, but looks good now that I noticed that. Tests are passing.

Other than suggesting the non-prefixed fsPromises, I would be 👍 to merging this.

[DeeDeeG]

(Getting sidetracked, feel free to skip this: To be honest, this entire file is suspect for being replaced with core fs module stuff -- once we get on newer Electron and Node versions, we should consider core fs.cp instead of ncp as well. But that requires Node v16.7.0 or newer and is still experimental, so no dice on that quite yet... And that's a bit too much scope creep for this one obviously applicable dependency replacement today.)

(A long term "wishlist item" is to get rid of wrench, too.)

[confused-Techie]

Thanks for taking a look at this one as well.

And great catch on the import name. But yeah otherwise this one felt easy enough and could get rid of the whole dep. But you are totally right that I bet (maybe after a bump or two) we can remove ncp as well, which would be great. Even from there, if we were dedicated to use only normal NodeJS fs we could remove fs-plus especially considering that all fs usage comes from the main file edited in this PR.

But once tests are happy after all the changes we can go ahead and merge this one!

[DeeDeeG]

[ EDIT: Okay, I guess it's just a network error. Probably a fluke, but updating yarn.lock still wouldn't hurt, IMO. ]

Bizarre error on Windows Node 16 CI run, Yarn's not happy:

> error Couldn't find package "rimraf@^3.0.2" required by "[email protected]" on the "npm" registry.

([email protected] definitely exists: https://www.npmjs.com/package/rimraf?activeTab=versions)

Maybe this is a fluke and never happens again. On the other hand, could be good to update the yarn.lock file and commit it in this PR? Maybe that helps?

[DeeDeeG]

Formally approving, but would recommend updating yarn.lock in this PR or I might do a follow-up PR just to update that, since it's used by core repo to resolve the dependencies, and should stay in sync or else core repo will keep updating it on-the-fly all the time, defeating the point of the lockfile.)

[DeeDeeG]

I can push commit 53d3d5f to this branch if you want an updated yarn.lock lockfile in this PR. Otherwise I can do a follow-up PR to sync yarn.lock. Also if you get to it without reading this, then no worries, lol.

[confused-Techie]

@DeeDeeG if you'd like feel free to push it, otherwise I can get an update in within the next 30 minutes

[DeeDeeG]

@confused-Techie done, I pushed the updated lockfile to this PR's branch just now.

[confused-Techie]

Thanks @DeeDeeG merging this one now then!