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

Different runtime behaviour of fs.realpath vs fs.promises.realpath is very confusing #37737

Open
bpasero opened this issue Mar 13, 2021 · 4 comments
Labels
fs Issues and PRs related to the fs subsystem / file system.

Comments

@bpasero
Copy link
Contributor

bpasero commented Mar 13, 2021

  • Version: 12.18.3
  • Platform: macOS
  • Subsystem: ARM

What steps will reproduce the bug?

Have a project that was still using promisify(fs.realpath) and replace this with fs.promises.realpath to clean it up thinking the runtime behaviour will be the same. But in reality there is different behaviour: on Windows, subst drives will be resolved to their targets when realpath.native is used and left untouched with realpath.

How often does it reproduce? Is there a required condition?

Always when using subst drives on Windows. There are probably more differences.

What is the expected behavior?

That the fs API is in sync with the non promised variant. fs.promises.realpath should be a drop in replacement for promisify(fs.realpath) but not promisify(fs.realpath.native). There should be a fs.promises.realpath.native.

What do you see instead?

fs.promises.realpath behaves like promisify(fs.realpath.native)

@aduh95
Copy link
Contributor

aduh95 commented Mar 13, 2021

FWIW fsPromises.realpath is documented as a promesified version of fs.realpath.native (https://nodejs.org/api/fs.html#fs_fspromises_realpath_path_options).

There should be a fs.promises.realpath.native.

Do you want to open a PR?

@aduh95 aduh95 added the fs Issues and PRs related to the fs subsystem / file system. label Mar 13, 2021
@bpasero
Copy link
Contributor Author

bpasero commented Mar 13, 2021

Do you want to open a PR?

Well, if there is agreement that fs.promises.realpath behaves like fs.realpath then I can do a PR.

@aduh95
Copy link
Contributor

aduh95 commented Mar 14, 2021

FWIW the reason the implementations differ was explained here: #15485 (comment)

There are some variations from the base fs module (for instance, fs.async.realpath() uses the faster libuv realpath implementation rather than the slower js implementation.

Why?

Performance mainly. And lack of backwards compatibility concerns. The key reason we ended up having to revert back to the old realpath implementation is because changing broke existing code. We don't have that issue here. Also, if we kept the js implementation, we wouldn't really gain any performance advantages over simply doing util.promisify(fs.realpath) given the way it is implemented.

@jasnell
Copy link
Member

jasnell commented Apr 30, 2021

This was an intentional decision. I'd be -1 to changing the current behavior of fs.promises.realpathto matchfs.realpath`.

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

No branches or pull requests

3 participants