remove fs-plus from image-view package

[sertonix]

This removes the fs-plus package from the dependencies of the image-view package.

[DeeDeeG]

One thing is that fs-plus generally ignores errors, whereas they may still be raised/thrown with the basic fs module from Node/Electron.

We are essentially only using fs.statSync() here, so we may want to do a little helper function that at least ignores ENOENT for when the given path doesn't exist? Or maybe ignore all errors if the rest of the file could handle that? Or at least log the errors somehow?

We also use fs.isFileSync() from fs-plus, which needs to be replaced -- see:
https://github.com/atom/fs-plus/blob/ba56f01d4f434982dff734186b3b6522eb28de75/src/fs-plus.mjs#L153-L164

Which could pretty easily be replaced by fs.statSync().isFile(), if we have figured out the error handling situation as above.

---

If this file reasonably will never call fs.statSync() on a file or path that doesn't exist, then we can just replace it with fs.statSync() as described above. Haven't looked closely enough to tell that much yet.

[sertonix]

That is wierd.
I havent changed the file where fs.isFileSync() is used. It already requires the normal fs.
That means there is most likely some unused code.

Edit: I messed up with uncommitet changes
I will take a closer look at the code.

[sertonix]

The problems should now be fixed

[DeeDeeG]

Thanks for replacingfs.isFileSync(). That part looks good to me. 👍

And sorry for multiple rounds of feedback hours apart.

---

I think we can safely use fs.existsSync() from the Node/Electron fs API, and without try/catch? -- I don't think that ever throws errors?

For spec/image-editor-status-view-spec.js and spec/image-editor-view-spec.js, the second commit removes fs.existsSync() in favor of fs.statSync() in a try{} block.

I would personally prefer to just go back to fs.existsSync() for the two spec files. I would approve at that point.

[sertonix]

You are right. That never throws. I changed that now

[confused-Techie]

LGTM!

Glad to see you guys were able to get everything else sorted, but otherwise as of now this looks great!

[DeeDeeG]

Approving because: This PR removes an unnecessary dependency, and replaces it with simpler stuff that looks just as functional, IMO.

I'll go ahead and merge this now.

Thank you!

[DeeDeeG]

Just want to specifically comment why this is fine 👍 :

The optional chaining thing with ? (object?.method()) on a declared variable won't throw even if the method doesn't exist.

Some docs: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Optional_chaining

If the object accessed or function called is undefined or null, it returns undefined instead of throwing an error.

Given let fileStats above, fileStats is declared. If some error happened with fs.statSync() above, and there is no fileStats.isFile() method, the if (fileStats?.isFile()) below will effectively be if(undefined), which is okay and doesn't throw. The undefined is just another falsy value.

(I did test this, also.)