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

Add pathExists() #406

Merged
merged 1 commit into from
Apr 25, 2017
Merged

Add pathExists() #406

merged 1 commit into from
Apr 25, 2017

Conversation

RyanZim
Copy link
Collaborator

@RyanZim RyanZim commented Apr 24, 2017

This is my approach at resolving #145.

fs.exists is broken. We're a drop-in replacement for fs, so we can't fix it.

We can make a new method that acts like fs.exists should have behaved all along. I've named it pathExists().

I also aliased fs.existsSync to fs.pathExistsSync for consistency.

Example Usage

const fs = require('fs-extra')

const file = '/tmp/this/path/does/not/exist/file.txt'
// Promise usage:
fs.pathExists(file)
  .then(exists => console.log(exists)) // => false
// Callback usage:
fs.pathExists(file, (err, exists) => {
  console.log(err) // => null
  console.log(exists) // => false
})

@coveralls
Copy link

Coverage Status

Coverage increased (+0.08%) to 86.812% when pulling 0e7d1b3 on path-exists into 33b7f84 on develop.

@manidlou
Copy link
Collaborator

LGTM! 👍

@jprichardson jprichardson merged commit 2e9bd24 into develop Apr 25, 2017
@jprichardson
Copy link
Owner

I like it, nice work @RyanZim!

@jaredallard
Copy link

I'm interested in why fs-extra doesn't support using the not-broken fs.exists when accessing it via a promise. Seems like you could make it drop-in compatible via the promise version and keep using the broken version when NOT accessing it via a promise. Maybe I'm missing something though

@RyanZim
Copy link
Collaborator Author

RyanZim commented Mar 22, 2018

@jaredallard We do promisify fs.exists, and still offer the broken fs.exists to callback users. However, we encourage the use of fs.pathExists, since it doesn't use deprecated Node APIs, and the callback version behaves correctly.

@jaredallard
Copy link

jaredallard commented Mar 22, 2018

@RyanZim Yes, I know fs.exists is promisified, but I'm wondering why not have it point to/use the solution that pathExists implements, since you can literally rewrite fs.exists -> fs.pathExists right now. Since how fs.exists behaves (in it's broken way) is somewhat how it's consumed as a promise (catch is used as the err instead).

TL;DR
The normal callback way should point to the broken one for compat, but the fs.exists promise version could be the same as the current fs.pathExists AFAIK (for people who don't upgrade to pathExists (which isn't a standard api anyways))

@RyanZim
Copy link
Collaborator Author

RyanZim commented Mar 22, 2018

The normal callback way should point to the broken one for compat, but the fs.exists promise version could be the same as the current fs.pathExists

Yes; that's the way things currently work.

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

Successfully merging this pull request may close these issues.

5 participants