Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fs.exists is deprecated #60

Closed
juanpicado opened this issue Jul 12, 2019 · 11 comments · Fixed by #159
Closed

fs.exists is deprecated #60

juanpicado opened this issue Jul 12, 2019 · 11 comments · Fixed by #159

Comments

@juanpicado
Copy link
Member

My reason:

We use fs.exists at least twice in our source code, this has been deprecated, we should upgrade it.

https://nodejs.org/api/fs.html#fs_fs_exists_path_callback

@juanpicado juanpicado transferred this issue from verdaccio/local-storage Aug 15, 2019
@juanpicado juanpicado pinned this issue Sep 29, 2019
juanpicado pushed a commit that referenced this issue Sep 29, 2019
@shrirambalaji
Copy link
Contributor

@juanpicado I'd like to take a dig at this issue.

@juanpicado
Copy link
Member Author

It's booked then :-)

@shrirambalaji
Copy link
Contributor

shrirambalaji commented Sep 30, 2019

@juanpicado The memory-fs library we are using doesn't seem to have an fs.access method. Although it has fs.stat, node docs don't seem recommend it:

Using fs.stat() to check for the existence of a file before calling fs.open(), fs.readFile() or fs.writeFile() is not recommended. Instead, user code should open/read/write the file directly and handle the error raised if the file is not available.

To check if a file exists without manipulating it afterwards, fs.access() is recommended.

Also, the library itself hasn't been updated in a while, so the chances of an upstream fix seem to be minimal.

Do you have a suggestion on what should be done? As mentioned, we could handle error the directly in the try-catch block without checking for the file's existence.

Or, If we should be using a different library,memfs looks like a good alternative.

@juanpicado
Copy link
Member Author

Here you can find some inspiration verdaccio/local-storage#174

Screen Shot 2019-10-01 at 7 49 09 AM

@shrirambalaji
Copy link
Contributor

@juanpicado Thanks for the pointers. I have a PR up for review :)

@DanielRuf
Copy link
Contributor

existsSync is not deprecated afaik.

https://nodejs.org/api/fs.html#fs_fs_existssync_path

@DanielRuf
Copy link
Contributor

Also it was deprecated since NodeJS 1.0.0
Bildschirmfoto 2019-10-05 um 11 38 13

@DanielRuf
Copy link
Contributor

fs.exists() is deprecated, but fs.existsSync() is not. The callback parameter to fs.exists() accepts parameters that are inconsistent with other Node.js callbacks. fs.existsSync() does not use a callback.

@DanielRuf DanielRuf changed the title fs.exists is deprecated at Node.js >12 fs.exists is deprecated Oct 5, 2019
@DanielRuf
Copy link
Contributor

@juanpicado
Copy link
Member Author

FYI #159

@DanielRuf
Copy link
Contributor

I already saw it and gave feedback there ;-)

Bildschirmfoto 2019-10-05 um 11 59 47

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants