emptyDir() callback arguments are inconsistent

[rahatarmanahmed]

When using emptyDir(), the arguments passed to the callback are inconsistent. When the directory doesn't exist, the arguments are [null, '/path/to/dir']. If the directory does exist, there are no arguments passed.

Reproduce with:

node -e "require('fs-extra').emptyDir('/path/to/nonexistent/directory', function() { console.log(arguments) })"
node -e "require('fs-extra').emptyDir('/path/to/nonexistent/directory', function() { console.log(arguments) })"

which outputs:

{ '0': null,
  '1': '/path/to/nonexistent/directory' }
{}

[rahatarmanahmed]

This seems to happen to ensureDir() as well. I think it happens for any function that has a logic of "mkdir" or "something else", because "mkdir" will return the dirname as an argument, but the "something else" doesn't return an argument.

[RyanZim]

Just a clarification:

emptyDir():

{ '0': null, '1': '/home/ryan/t/fs-ext/noop' }
{}

ensureDir():

{ '0': null, '1': '/home/ryan/t/fs-ext/noop' }
{ '0': null, '1': null }

[RyanZim]

@jprichardson What is the correct arguments signature?

[jprichardson]

When in doubt, we should:

1. Follow the lead of Node.js.
2. Strive for consistency.

These aren't hard rules though, especially if it means dirtying up the code to ensure that err is either undefined always or null (assuming no Error).

@rahatarmanahmed can you elaborate more on the problem this is causing your code?

[rahatarmanahmed]

@jprichardson it causes issues when used with a package like run-waterfall. I ran into it while working on this PR: electron/packager@72ab0e7

[RyanZim]

If @rahatarmanahmed wants to submit a PR, SGTM. Suffice to say this isn't high-priority for me.

[rahatarmanahmed]

Sorry, I only ran into this working on a PR for another project. I don't personally use fs-extra or run-waterfall, so it's not a high-priority for me either 😢 . (Plus it's not entirely a blocker, since there is a workaround).

[RyanZim]

I don't think this is going to get fixed for v1.0.0, so removing the milestone.

@jprichardson If you want to close this as wontfix, IDK.

[jprichardson]

@rahatarmanahmed interested in submitting a PR?

[rahatarmanahmed]

@jprichardson Not really, sorry. Hate to be one of those hit-and-run issue creators, but I don't really have an investment in this project.

[jprichardson]

@rahatarmanahmed I appreciate your candid response :)

[RyanZim]

Going to close this as a wont-fix; not saying this will never change, but I don't see myself devoting time to this edge case in the near future. If someone stumbles across this again, PR welcome.