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

path.relative(dir, dir) returns empty string #4826

Closed
eush77 opened this issue Jan 23, 2016 · 5 comments
Closed

path.relative(dir, dir) returns empty string #4826

eush77 opened this issue Jan 23, 2016 · 5 comments
Labels
path Issues and PRs related to the path subsystem.

Comments

@eush77
Copy link

eush77 commented Jan 23, 2016

path.relative returns empty string when its from and to arguments are identical.

> path.relative(dir, dir)
''

Since '' is not a valid path, this can blow up:

function ls (dir) {
  dir = path.relative(process.cwd(), dir);
  console.log(dir);
  // ...

  console.log(fs.readdirSync(dir));
}

This function ls works most of the time (nonexistent files and permissions aside), but not if dir points to the working directory:

> ls('.')

Error: ENOENT: no such file or directory, scandir ''
    at Error (native)
    at Object.fs.readdirSync (fs.js:856:18)
    at ls (repl:6:16)

I find the fact that path.relative returns a valid path, except in one corner case when the two arguments are identical, rather inconvenient.

However, this is probably intentional since there is a test for this introduced in #2106. But I haven't found any reasoning for this, and the reasoning of #2106 itself is not (directly) applicable here: it's one thing to defensively treat empty strings as '.', and the other to return an empty string instead of a valid path.

Expected bahavior:

> path.relative(dir, dir)
'.'
@silverwind silverwind added the path Issues and PRs related to the path subsystem. label Jan 23, 2016
@silverwind
Copy link
Contributor

I see what you mean, but '.' means 'current directory' to me, something which does not seem correct for me for arbitrary paths which are equal. It would only be correct when dir equals $PWD.

@eush77
Copy link
Author

eush77 commented Jan 24, 2016

I see what you mean, but '.' means 'current directory' to me, something which does not seem correct for me for arbitrary paths which are equal. It would only be correct when dir equals $PWD.

That's not actually correct. Every directory has '.' and '..', and they can appear anywhere in the path:

$ cd /tmp
$ cd ../var/./opt/../../.
$ pwd
/

@zeusdeux
Copy link
Contributor

Shouldn't path.relative(dir, dir) just return dir?

@silverwind
Copy link
Contributor

Shouldn't path.relative(dir, dir) just return dir?

No, that would imply there's another sub-directory like /dir/dir when dir = '/dir'.

That's not actually correct. Every directory has '.' and '..', and they can appear anywhere in the path:

Right, I see returning '.' has some uses, like process.chdir(path.relative(process.cwd(), 'dir')). Still, I'm uncomfortable with breaking documented behaviour.

@evanlucas
Copy link
Contributor

This is documented at https://nodejs.org/dist/latest-v5.x/docs/api/path.html#path_path_relative_from_to.

I'll go ahead and close. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
path Issues and PRs related to the path subsystem.
Projects
None yet
Development

No branches or pull requests

4 participants