`exists` is deprecated

[ursi]

purescript-node-fs/src/Node/FS/Async.purs

Line 84 in 02d610f

, exists :: forall a. Fn2 FilePath (Boolean -> a) Unit

https://nodejs.org/dist/latest-v14.x/docs/api/fs.html#fs_fs_exists_path_callback

[thomashoneyman]

@ursi Thanks for catching this. It appears that this was deprecated back in Node v1.0.0, and we only support versions back to Node v10.0.0 (here's the docs link for that version).

The Node documentation suggests using one of these functions instead:

• https://nodejs.org/dist/v10.0.0/docs/api/fs.html#fs_fs_stat_path_callback
• https://nodejs.org/dist/v10.0.0/docs/api/fs.html#fs_fs_access_path_mode_callback

We already have stat supported in this library, but not access. I would accept a PR which removes exists from this library because it has been deprecated for many versions, and ideally we'd also add bindings to access.

[thomashoneyman]

If we don't get this in by the time the next major release goes out, then we can issue a patch release that deprecates (but doesn't remove) the exists function by providing a warning that indicates users should reach for stat or access instead.

[JordanMartinez]

This is kind of a weird one. Node says that one should check if a file exists and, if so, then write to it but just calling fs.writeFile and handling a potential error. If an error occurs, then the file did not exist. If it doesn't, the file exists. Doing an exists check and then writing to the file can produce a race condition. (See https://nodejs.org/dist/latest-v16.x/docs/api/fs.html#fsexistspath-callback)

[thomashoneyman]

I think exists should be removed. It's been deprecated since Node 1 and its use is discouraged.

[JordanMartinez]

So, typically exists is used to check whether a file is there first, and then read/write to that file if it is. However, using either stat or access, the two functions Node docs recommend in place of exist both state that they shouldn't be used in this way.

Rather, they say to try to read or write to the file and handle any errors if the file doesn't exist. So, it's not so much that this can't be removed, but that migrating code to something that works correctly (i.e. no race conditions) without it is painful.

[JordanMartinez]

Or put differently, it would be better to provide a more opinionated API like readIfExists "filePath" or writeIfExists.

[JordanMartinez]

Ah, thanks for pointing that out. I guess the race condition is only because exists uses a callback. Since existsSync doesn't use callbacks, there is no such issue.

[ursi]

Oh, I had deleted the comment before you replied (at least on my end). I realized existsSync is already in this library, in the form of Node.FS.Sync.exists.

[JordanMartinez]

Right, but my concern with removing exists was not realizing that something of its equivalence could be done. Since existsSync exists, this issue is just waiting for someone to submit a PR.