Skip to content

Deprecate fs.exists or fix its API #103

Closed
Closed
Deprecate fs.exists or fix its API#103
Assignees
cjihrig
Labels
fsIssues and PRs related to file-system APIs and the fs module.

Description

@benjamingr
benjamingr
opened on Dec 6, 2014

fs.exists is infamous for having an inconsistent non-nodeback API that confuses new users often and can be a pain spot.

I see two good alternatives:

  • Deprecate it, mark it as deprecated in the docs and suggest fs.stat instead + discuss the inherent problem with using exists (race condition). Add a big warning. Optionally console.log a deprecation notice when the server is first started.
  • Change the API to return a nodeback. I like this idea less since it'd have to be written in a way that wouldn't break current code. This sounds harder and less optimal.

Personally I'm for the first. Let's clean up fs :)

Activity

  1. rvagg commented on Dec 6, 2014

    @rvagg
    rvagg
    on Dec 6, 2014
    Member

    Yes! Option 1 is my vote. This is a tiny black spot on the Node fs API.

  2. cjihrig commented on Dec 7, 2014

    @cjihrig
    cjihrig
    on Dec 7, 2014
    Contributor

    +1 for deprecating. This was already being discussed in nodejs/node-v0.x-archive#8418. fs.access() an alternative is also up for PR nodejs/node-v0.x-archive#8714. Since both of those are mine, I'd be glad to do the work here.

  3. ghostbar commented on Dec 7, 2014

    @ghostbar
    ghostbar
    on Dec 7, 2014
    Contributor

    +1 on option 1.

  4. juliangruber commented on Dec 7, 2014

    @juliangruber
    juliangruber
    on Dec 7, 2014
    Member

    👍 for deprecating

  5. bnoordhuis commented on Dec 7, 2014

    @bnoordhuis
    bnoordhuis
    on Dec 7, 2014
    Member

    Sounds reasonable. @cjihrig Go for it.

  6. benjamingr commented on Dec 7, 2014

    @benjamingr
    benjamingr
    on Dec 7, 2014
    MemberAuthor

    @cjihrig yes please! The fix looks pretty solid and so far everyone is for deprecating it.

  7. fengmk2 commented on Dec 7, 2014

    @fengmk2
    fengmk2
    on Dec 7, 2014
    Contributor

    @cjihrig 👍 here

  8. indutny commented on Dec 8, 2014

    @indutny
    indutny
    on Dec 8, 2014
    Member

    Summoning @caineio

  9. caineio commented on Dec 8, 2014

    @caineio
    caineio
    on Dec 8, 2014

    Hello!

    I am pleased to see your valuable contribution to this project. Would you
    please mind answering a couple of questions to help me classify this submission
    and/or gather required information for the core team members?

    Questions:

    1. Issue-only Does this issue happen in core, or in some user-space
      module from npm or other source? Please ensure that the test case
      that reproduces this problem is not using any external dependencies.
      If the error is not reproducible with just core modules - it is most
      likely not a io.js problem. Expected: yes
    2. Which part of core do you think it might be related to?
      One of: tls, crypto, buffer, http, https, assert, util, streams, smalloc, cluster, child_process, dgram, c++, docs, other (label)
    3. Which versions of io.js do you think are affected by this?
      One of: v0.10, v0.12, v1.0.0 (label)

    Please provide the answers in an ordered list like this:

    1. Answer for the first question
    2. Answer for the second question
    3. ...

    Note that I am just a bot with a limited human-reply parsing abilities,
    so please be very careful with numbers and don't skip the questions!

    In case of success I will say: ...summoning the core team devs!.

    In case of validation problem I will say: Sorry, but something is not right here:.

    Truly yours,
    Caine.

    Responsibilities

    1. indutny: crypto, tls, https, child_process, c++
    2. trevnorris: buffer, http, https, smalloc
    3. bnoordhuis: http, cluster, child_process, dgram
  10. caineio
    added
    need info
    on Dec 8, 2014
  11. piscisaureus
    assigned
    cjihrig
    on Dec 8, 2014
  12. piscisaureus
    added
    wipIssues and PRs that are still a work in progress.
    on Dec 8, 2014

  13. benjamingr commented on Dec 8, 2014

    @benjamingr
    benjamingr
    on Dec 8, 2014
    MemberAuthor
    1. Yes
    2. fs
    3. v0.12

    On Dec 8, 2014, at 15:30, Michael Caine notifications@github.com wrote:

    Hello!

    I am pleased to see your valuable contribution to this project. Would you
    please mind answering a couple of questions to help me classify this submission
    and/or gather required information for the core team members?

    Questions:

    Issue-only Does this issue happen in core, or in some user-space module from npm or other source? Please ensure that the test case that reproduces this problem is not using any external dependencies. If the error is not reproducible with just core modules - it is most likely not a io.js problem. Expected: yes
    Which part of core do you think it might be related to? One of: tls, crypto, buffer, http, https, assert, util, streams, smalloc, cluster, child_process, dgram, c++, docs, other (label)
    Which versions of io.js do you think are affected by this? One of: v0.10, v0.12, v1.0.0 (label)
    Please provide the answers in an ordered list like this:

    Answer for the first question
    Answer for the second question
    ...
    Note that I am just a bot with a limited human-reply parsing abilities,
    so please be very careful with numbers and don't skip the questions!

    In case of success I will say: ...summoning the core team devs!.

    In case of validation problem I will say: Sorry, but something is not right
    here:.

    Truly yours,
    Caine.

    Responsibilities

    indutny: crypto, tls, https, child_process, c++
    trevnorris: buffer, http, https, smalloc
    bnoordhuis: http, cluster, child_process, dgram
    —
    Reply to this email directly or view it on GitHub.

  14. indutny commented on Dec 8, 2014

    @indutny
    indutny
    on Dec 8, 2014
    Member

    @caineio what's up with you? Why are you ignoring this?

  15. indutny
    unassigned
    cjihrig
    on Dec 8, 2014

  16. 5 remaining items

  17. caineio
    added
    fsIssues and PRs related to file-system APIs and the fs module.
    v0.12
    on Dec 8, 2014
  18. indutny
    assigned
    cjihrig
    on Dec 8, 2014
  19. cjihrig
    added a commit that references this issue on Dec 19, 2014

    fs: deprecate exists() and existsSync()

    5678595

  20. cjihrig commented on Dec 19, 2014

    @cjihrig
    cjihrig
    on Dec 19, 2014
    Contributor

    fs.exists() and fs.existsSync() are deprecated as of 5678595

  21. cjihrig
    closed this as completedon Dec 19, 2014

  22. benjamingr commented on Dec 19, 2014

    @benjamingr
    benjamingr
    on Dec 19, 2014
    MemberAuthor

    Awesome news :) Thanks a ton.

  23. cjihrig
    added a commit that references this issue on Jan 12, 2015

    fs: undeprecate exists() and existsSync()

    3a85eac

  24. matthew-dean commented on Apr 30, 2015

    @matthew-dean
    matthew-dean
    on Apr 30, 2015

    I am sad at this deprecation.

  25. captn3m0 commented on May 3, 2015

    @captn3m0
    captn3m0
    on May 3, 2015

    We have an npm module now: https://github.com/sindresorhus/path-exists

  26. Mark-Simulacrum
    added a commit that references this issue on Jun 25, 2015

    Use path-exists instead of fs.exists.

    b308602

  27. xixixao commented on Oct 10, 2015

    @xixixao
    xixixao
    on Oct 10, 2015

    /facepalm How was this not sufficient:

    fs.exists() should not be used to check if a file exists before calling fs.open(). Doing so introduces a race condition since other processes may change the file's state between the two calls. Instead, user code should call fs.open() directly and handle the error raised if the file is non-existent.

  28. matthew-dean commented on Oct 10, 2015

    @matthew-dean
    matthew-dean
    on Oct 10, 2015

    Because I don't want to open the file.

  29. Fishrock123 commented on Oct 10, 2015

    @Fishrock123
    Fishrock123
    on Oct 10, 2015
    Contributor

    Please see: #1592

  30. nodejs
    locked and limited conversation to collaborators on Oct 10, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

fsIssues and PRs related to file-system APIs and the fs module.

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions